copy_to_user not working in kernel module

Question

I was trying to use copy_to_user in kernel module read function, but am not able to copy the data from kernel to user buffer. Please can anyone tell me if I am doing some mistake. My kernel version is 2.6.35. I am giving the portion of kernel module as well as the application being used to test it. Right now my focus is why this copy_to_user is not working. Any help will great.

///////////////////////////////////kernel module//////////////////////////////////////

#define BUF_LEN 80

static char msg[BUF_LEN];       
static char *msg_Ptr;

static int device_open(struct inode *inode, struct file *file)
{
static int counter = 0;

if (Device_Open)
    return -EBUSY;

Device_Open++;
printk(KERN_ALERT "In open device call\n");

sprintf(msg, "I already told you %d times Hello world!\n", counter++);
msg_Ptr = msg;
try_module_get(THIS_MODULE);

return SUCCESS;
}


static ssize_t device_read(struct file *filp,    
           char __user *buffer,    
           size_t length,    
           loff_t * offset)
{
/*
 * Number of bytes actually written to the buffer 
 */
int bytes_read = 0;

/*
 * If we are at the end of the message, 
 * return 0 signifying end of file 
 */
if (*msg_Ptr == 0)
    return 0;

/* 
 * Actually put the data into the buffer 
 */


else {
    bytes_read=copy_to_user(buffer, msg, length);
    if (bytes_read==-1);
        {
         printk(KERN_INFO "Error in else while copying the data \n");
        }

    }

   return bytes_read;
}






////////////////////////////////////////application////////////////////////
#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h> 
#include <stdlib.h>

#define  BUF_SIZE    40

int main()
{
ssize_t num_bytes;
int fd, n=0;
char buf[BUF_SIZE];

fd=open("/dev/chardev", O_RDWR);
if(fd== -1){perror("Error while opening device");exit(1);}

printf("fd=%d\n",fd);
num_bytes=read(fd, buf, BUF_SIZE);
if(num_bytes==-1){perror("Error while reading"); exit(2);}

printf("The value fetched is %lu bytes\n", num_bytes);

while(n<=num_bytes)
    {
        printf("%c",buf[n]);
        n++;
    }

close(fd);
return 0;

}

score 2 · Accepted Answer · edited May 23 '17 at 12:13

There are a few problems in the code snippet you wrote. First of all, it is not a good thing to make the call try_module_get(THIS_MODULE);
This statement tries to increase the refcount of the module ... in the module itself ! Instead, you should set the owner field of the file_ops structure to THIS_MODULE in your init method. This way, the reference handling will happen outside the module code, in the VFS layer. You might take a look at Linux Kernel Modules: When to use try_module_get / module_put.
Then, as it was stated by Vineet you should retrieve the pointer from the file_ops private_data field.

And last but not least, here is the reason why it seems an error happened while ... Actually ... It did not : The copy_to_user call returns 0 if it has successfully copied all the desired bytes into the destination memory area and a strictly positive value stating the number of bytes that were NOT copied in case of error. That said, when you run :

/* Kernel part */
bytes_read=copy_to_user(buffer, msg, length);
/* 
 * Wrong error checking :
 * In the below statement, "-1" is viewed as an unsigned long.
 * With a simple equality test, this will not bother you
 * But this is dangerous with other comparisons like "<" or ">"
 * (unsigned long)(-1) is at least 2^32 - 1 so ...
 */
if (-1 == bytes_read) {
    /* etc. */
}
return bytes_read;

/* App part */
num_bytes=read(fd, buf, BUF_SIZE);
/* etc.. */
while(n<=num_bytes) {
    printf("%c",buf[n]);
    n++;
}

You should only get one character upon a successful copy, that is only a single "I" in your case. Moreover, you use your msg_Ptr pointer as a safeguard but you never update it. This might result in a wrong call to copy_to_user.
copy_to_user checks the user-space pointer with a call to access_ok, but if the kernel-space pointer and the given length are not allright, this might end in a Kernel Oops/Panic.

Thanks Rerito, it solved my problem. I was doing a silly mistake of return value. I would definitely keep in mind the tips given by you. Thanks a lot — sandy, Jan 25 '13 at 06:29
Hey Rerito, still I have one doubt. I am getting the data in application but when I check in dmesg I get the print for error check even though when I have modified the check as mentioned by you. if (-1 == bytes_read). I have checked the value of bytes read in the same printk and found it 0. Can you please explain this behaviour — sandy, Jan 25 '13 at 06:53
Hey Rerito I found the error was in programming. Sorry for bothering you. Thanks for all your help — sandy, Jan 25 '13 at 06:57
That was may be the semi colon at the end of your "if" statement ? — Rerito, Jan 25 '13 at 08:01
I might also advice you to get used to the Linux Kernel coding rules. This will : 1. Make your code easier to read for others 2. Sometimes it will prevent you from making little mistakes that can screw up the whole code You can find these rules at http://www.kernel.org/doc/Documentation/CodingStyle . — Rerito, Jan 25 '13 at 08:30

score 0 · Answer 2 · answered Jan 24 '13 at 07:43

0

I think you should update the file->private_data in open and then you have to fetch that in your structure. Because I guess the msg buffer ( kernel buffer ) is not getting proper refernce.

answered Jan 24 '13 at 07:43

Vineet Saddyan

11
6

But instead of copy user if I am using put_user, the things work fine and I receive the data through application while (length && *msg_Ptr) { put_user(*(msg_Ptr++), buffer++); length--; bytes_read++;} – sandy Jan 24 '13 at 07:48

copy_to_user not working in kernel module

2 Answers2