LinuxQuestions.org
Visit Jeremy's Blog.
Go Back   LinuxQuestions.org > Forums > Non-*NIX Forums > Programming
User Name
Password
Programming This forum is for all programming questions.
The question does not have to be directly related to Linux and any language is fair game.

Notices


Reply
  Search this Thread
Old 02-14-2012, 10:16 AM   #1
dennisdd
Member
 
Registered: Oct 2011
Posts: 37

Rep: Reputation: Disabled
Linux kernel copy_to_user to user space display different result


There some bugs reading from the user space with this application. Is my copy_to_user method not correct?

The following is the readout from terminalwhich is wrong)

Press r to read from device or w to write the device r
0x-1075024108 0x15123440 0xe70401 0xe6f8dc 0xe73524
0x0 0x15037588 0xbfec6f14 0xe57612 0xbfec6f34
0x15037140 0x2 0xe57334 0xc6d690 0xd696910
0x-1075024080 0x15071734 0xc737c9 0x804835a 0x2

The following is the code from apps layer:

read(fd, read_buf, sizeof(read_buf));

for(i=0;i<=(BUFF_SIZE / sizeof(int));i+=5)
printf(" 0x%x 0x%x 0x%x 0x%x 0x%x \n",
read_buf[i],read_buf[i+1],read_buf[i+2],
read_buf[i+3],read_buf[i+4]);
break;

and the following is my driver code:

#include <linux/version.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/fs.h>
#include <linux/cdev.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>
#include <linux/mm.h>
#ifdef MODVERSIONS
# include <linux/modversions.h>
#endif
#include <asm/io.h>
#include <asm/uaccess.h> // required for copy_from and copy_to user

/* character device structures */
static dev_t mmap_dev;
static struct cdev mmap_cdev;

/* methods of the character device */
static int mmap_open(struct inode *inode, struct file *filp);
static int mmap_release(struct inode *inode, struct file *filp);


/* the file operations, i.e. all character device methods */
static struct file_operations mmap_fops = {
.open = mmap_open,
.release= mmap_release,
.owner = THIS_MODULE,
};
static int *vmalloc_area;

#define NPAGES 1//16
#define BUFF_SIZE 64 // bytes

/* character device open method */
static int mmap_open(struct inode *inode, struct file *filp)
{
return 0;
}

/* character device last close method */
static int mmap_release(struct inode *inode, struct file *filp)
{
return 0;
}

ssize_t read(struct file *filp, int *buff, size_t count, loff_t *offp)
{
unsigned long bytes_left;

printk("Inside read \n");

bytes_left = copy_to_user(buff, vmalloc_area , count);

if(bytes_left<0)
bytes_left = -EFAULT;

return bytes_left;
}

/* module initialization - called at module load time */
static int __init membuff_init(void)
{
int ret = 0, i =0;

printk(KERN_ERR "@membuff_init\n");

/* allocate a memory area with vmalloc. */
if ((vmalloc_area = vmalloc(BUFF_SIZE)) == NULL) {
ret = -ENOMEM;
goto out_vfree;
}

/* get the major number of the character device */
if( (ret = alloc_chrdev_region(&mmap_dev, 0, 1, "mmap")) < 0) {
printk(KERN_ERR "@membuff_init could not allocate major number for mmap\n");
goto out_vfree;
}
printk(KERN_ERR "@membuff_init Major number for mmap: %d\n",MAJOR(mmap_dev));

/* initialize the device structure and register the device with the kernel */
cdev_init(&mmap_cdev, &mmap_fops);
if ((ret = cdev_add(&mmap_cdev, mmap_dev, 1)) < 0) {
printk(KERN_ERR "@membuff_init could not allocate chrdev for mmap\n");
goto out_unalloc_region;
}

for (i = 0; i < (BUFF_SIZE / sizeof(int)); i +=1) {
vmalloc_area[i] = i;
printk(KERN_ERR "@membuff_init: %d\n",vmalloc_area[i]);
}
return ret;

out_unalloc_region:
unregister_chrdev_region(mmap_dev, 1);
out_vfree:
if(vmalloc_area)
vfree(vmalloc_area);
return ret;
}

/* module unload */
static void __exit mmap_exit(void)
{
if(vmalloc_area)
vfree(vmalloc_area);

/* remove the character deivce */
cdev_del(&mmap_cdev);
unregister_chrdev_region(mmap_dev, 1);

printk(KERN_ERR "@mmap_exit\n");
}

module_init(membuff_init);
module_exit(mmap_exit);
MODULE_DESCRIPTION("trying out copy_to_user");
MODULE_LICENSE("Dual BSD/GPL");
 
Old 02-15-2012, 05:42 AM   #2
Nominal Animal
Senior Member
 
Registered: Dec 2010
Location: Finland
Distribution: Xubuntu, CentOS, LFS
Posts: 1,723
Blog Entries: 3

Rep: Reputation: 948Reputation: 948Reputation: 948Reputation: 948Reputation: 948Reputation: 948Reputation: 948Reputation: 948
It is very difficult to read your code; please, use [CODE]...[/CODE] tags around your code.

That said, your read() function always returns the start of your vmalloc_area buffer.

Try this instead:
Code:
ssize_t read(struct file *filp, char __user *buff, size_t count, loff_t *offp)
{
    char   *source = (char *)vmalloc_area;
    size_t  length = BUFF_SIZE;

    /* Past end of data? */
    if (*offp >= length)
        return 0;

    /* Calculate the desired data region. */
    length -= (size_t)(*offp);
    source += (size_t)(*offp);

    /* Limit to the amount the user requested. */
    if (count > length)
        length = count;

    /* Copy the desired part to userspace */
    if (copy_to_user(buff, source, length))
        return -EFAULT; /* invalid pointer */

    /* Update the file pointer */
    *offp += (loff_t)length;

    /* Return the number of bytes read */
    return (ssize_t)length;
}
The fact that you use an int pointer for vmalloc_area makes pointer calculations difficult (as adding 1 to the pointer moves it by one int). To avoid that, I use a temporary pointer.

You might also wish to implement the llseek method,
Code:
loff_t llseek(struct file *filp, loff_t off, int whence)
{
    loff_t pos = filp->f_pos;

    switch (whence) {
    case 0: /* SEEK_SET */
        pos = off;
        break;
    case 1: /* SEEK_CUR */
        pos += off;
        break;
    case 2: /* SEEK_END */
        pos = (loff_t)BUFF_SIZE + off;
        break;
    default: /* Invalid, should never happen */
        return -EINVAL;
    }

    if (pos < (loff_t)0)
        return -EINVAL;

    filp->f_pos = pos;
    return pos;
}
so that you can use lseek(), fseek(), ftell() and rewind() with the device too.

The above code is untested. I'm very interested to know if it solves your problem -- you didn't exactly tell us what your problem was, just that you have a problem.
 
Old 02-15-2012, 08:01 AM   #3
dennisdd
Member
 
Registered: Oct 2011
Posts: 37

Original Poster
Rep: Reputation: Disabled
Some updates...I change a bit to debug my read() function
Code:
//#include <linux/config.h>
#include <linux/version.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/fs.h>
#include <linux/cdev.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>
#include <linux/mm.h>
#ifdef MODVERSIONS
#  include <linux/modversions.h>
#endif
#include <asm/io.h>
#include <asm/uaccess.h> // required for copy_from and copy_to user 

/* character device structures */
static dev_t mmap_dev;
static struct cdev mmap_cdev;

/* methods of the character device */
static int mmap_open(struct inode *inode, struct file *filp);
static int mmap_release(struct inode *inode, struct file *filp);
static ssize_t write(struct file *filp, const char __user *user_buf, size_t len, loff_t *offset);
static ssize_t read(struct file *filp, char __user *user_buf, size_t len, loff_t *offset);


/* the file operations, i.e. all character device methods */
static struct file_operations mmap_fops = {
        .open	= mmap_open,
        .release= mmap_release,
	.read	= read,
	.write	= write,
        .owner	= THIS_MODULE,
};
//static int *vmalloc_area;
static char *input_msg; 

#define NPAGES		1//16
#define BUFF_SIZE	64 // bytes
#define ONE_FRAME	960 // 960 bytes
#define MESSAGE_LENGTH	1024/ONE_FRAME

/* character device open method */
static int mmap_open(struct inode *inode, struct file *filp)
{
        return 0;
}

/* character device last close method */
static int mmap_release(struct inode *inode, struct file *filp)
{
        return 0;
}

static ssize_t write(struct file *filp, const char __user *user_buf, size_t len, loff_t *offset)
{
        int bytes = 0;

	if(len>MESSAGE_LENGTH)
		len=MESSAGE_LENGTH;
        
	bytes = copy_from_user(input_msg, user_buf, len);
	//bytes = copy_from_user(vmalloc_area, user_buf, len);

        if (bytes < 0) {
                return -EFAULT;
        }
        input_msg[len] = '\0';


        return bytes;
}


static ssize_t read(struct file *filp, char __user *user_buf, size_t len, loff_t *offset)
{
	unsigned long bytes_left = 0;
	int i = 0;

	printk("Inside read \n");

	if(len>MESSAGE_LENGTH)
		len = MESSAGE_LENGTH;

	printk(KERN_ERR "@membuff_init len: %d\n",len);
	
	for (i = 0; i <  len; i +=1) {
		input_msg[i] = i;
		printk(KERN_ERR "@membuff_init input_msg[%d]: %d\n",i,input_msg[i]);
        }

	bytes_left = copy_to_user(user_buf, input_msg , len);
//	bytes_left = copy_to_user(user_buf, vmalloc_area , len);

	if(bytes_left<0)
		bytes_left =  -EFAULT;

	return bytes_left;
}

/* module initialization - called at module load time */
static int __init membuff_init(void)
{
        int ret = 0,i = 0;

	printk(KERN_ERR "@membuff_init\n");

        /* allocate a memory area with vmalloc. */
/*	if ((vmalloc_area = vmalloc(BUFF_SIZE)) == NULL) {
                ret = -ENOMEM;
                goto out_vfree;
        } */

        input_msg = (char *)kmalloc(MESSAGE_LENGTH+1,GFP_KERNEL); ///somehow must be in kmalloc
        if (!input_msg) {
                return -ENOMEM;
        } 

        /* get the major number of the character device */
        if( (ret = alloc_chrdev_region(&mmap_dev, 0, 1, "mmap")) < 0) {
                printk(KERN_ERR "@membuff_init could not allocate major number for mmap\n");
                goto out_vfree;
        }
	printk(KERN_ERR "@membuff_init Major number for mmap: %d\n",MAJOR(mmap_dev));

        /* initialize the device structure and register the device with the kernel */
        cdev_init(&mmap_cdev, &mmap_fops);
        if ((ret = cdev_add(&mmap_cdev, mmap_dev, 1)) < 0) {
                printk(KERN_ERR "@membuff_init could not allocate chrdev for mmap\n");
                goto out_unalloc_region;
        }

	//for (i = 0; i < (BUFF_SIZE / sizeof(int)); i +=1) {
/*	for (i = 0; i < (BUFF_SIZE); i +=1) {
		input_msg[i] = i;
		printk(KERN_ERR "@membuff_init: %d\n",input_msg[i]);
        } */
        return ret;
	
out_unalloc_region:
        unregister_chrdev_region(mmap_dev, 1);
out_vfree:
	/*if(vmalloc_area)
        	vfree(vmalloc_area);*/
	if(input_msg)
		kfree(input_msg);
        return ret;
}

/* module unload */
static void __exit mmap_exit(void)
{
	/*if(vmalloc_area)
        	vfree(vmalloc_area);*/

	if(input_msg)
		kfree(input_msg); 

        /* remove the character deivce */
        cdev_del(&mmap_cdev);
        unregister_chrdev_region(mmap_dev, 1);

	printk(KERN_ERR "@mmap_exit\n");
}

module_init(membuff_init);
module_exit(mmap_exit);
MODULE_DESCRIPTION("trying out copy_to_user");
MODULE_LICENSE("Dual BSD/GPL");
Below is my application code
Code:
	char *buffer;
				//Allocate memory
				buffer=(char *)malloc(100);

				if (!buffer)
				{
					fprintf(stderr, "Memory error!");
					break;
				}
				else
				{
					read(fd, buffer, sizeof(buffer));
					for(i = 0;i <100;i++)
						printf("no:%d %d\n", i,buffer[i]);
			
					free(buffer);
				}
				break;
1st seem like when I do read() for 100bytes at the kernel side it is read as len=1.

terminal dump:
[ 4280.825279] @membuff_init Major number for mmap: 250
[ 4288.045512] Inside read
[ 4288.045530] @membuff_init len: 1
[ 4288.045543] @membuff_init input_msg[0]: 0

also the output from buffer at application layer:
The data in the device is
no:0 0
no:1 75
no:2 113
no:3 -73
no:4 46
no:5 78
no:6 61
no:7 -10
no:8 0
....

Did I missed something?
 
Old 02-15-2012, 08:17 AM   #4
Nominal Animal
Senior Member
 
Registered: Dec 2010
Location: Finland
Distribution: Xubuntu, CentOS, LFS
Posts: 1,723
Blog Entries: 3

Rep: Reputation: 948Reputation: 948Reputation: 948Reputation: 948Reputation: 948Reputation: 948Reputation: 948Reputation: 948
In your userspace code:
Quote:
Originally Posted by dennisdd View Post
Code:
buffer=(char *)malloc(100);
:
read(fd, buffer, sizeof(buffer));
Since buffer is a char pointer, sizeof (buffer) == sizeof (char *) which evaluates to 4 on 32-bit platforms, and to 8 on 64-bit platforms. This is not the cause of this problem, but please fix this or it will bite you later on.

Just use 100 as the size instead, or use size_t bufferlen = 100; buffer = (char *)malloc(bufferlen); to keep the actual size, also supplying bufferlen as the third parameter to the write() function.

The problem at hand:

In the kernel driver, you have
Quote:
Originally Posted by dennisdd View Post
Code:
#define ONE_FRAME	960 // 960 bytes
#define MESSAGE_LENGTH	1024/ONE_FRAME
which means that MESSAGE_LENGTH evaluates to 1. Because of the check
Code:
if(len>MESSAGE_LENGTH) len=MESSAGE_LENGTH;
in the write method in the kernel driver, your len will always be 1.


Note: You really should use parentheses around macros, i.e. (1024/ONE_FRAME) to avoid reordering issues. For example,
Code:
#define SUM(a,b) a+b
#define EQUATION(a,b,c) SUM(a,b)*c
is equivalent to
Code:
#define EQUATION(a,b,c) ( (a) + (b) * (c) )

Last edited by Nominal Animal; 02-15-2012 at 08:20 AM.
 
Old 02-15-2012, 08:24 AM   #5
dennisdd
Member
 
Registered: Oct 2011
Posts: 37

Original Poster
Rep: Reputation: Disabled
nominal animal,


"your read() function always returns the start of your vmalloc_area buffer."

On application code if I do following it will only return the input_msg[0] only?
Isnt that by telling the function sizeof(buffer) will indicate how much need to be call from the kernel memory? say like from location 0 -100th?

how do

Code:
fd = open("node", O_RDWR);
buffer=(char *)malloc(100);
read(fd, buffer, sizeof(buffer));

Code:
static ssize_t read(struct file *filp, char __user *user_buf, size_t len, loff_t *offset)
{
	unsigned long bytes_left = 0;
	int i = 0;
	size_t  length =MESSAGE_LENGTH;

	printk("Inside read \n");

	if(len>length)
		len =length;

	printk(KERN_ERR "@membuff_init len: %d\n",len);

	/* Calculate the desired data region. */
	length -= (size_t)(*offset);
	input_msg += (size_t)(*offset);
	
	for (i = 0; i <  len; i +=1) {
		input_msg[i] = i;
		printk(KERN_ERR "@membuff_init input_msg[%d]: %d\n",i,input_msg[i]);
        }

	bytes_left = copy_to_user(user_buf, input_msg , len);
//	bytes_left = copy_to_user(user_buf, vmalloc_area , len);

	/* Update the file pointer */
	*offset += (loff_t)length;

	/* Return the number of bytes read */
	return (ssize_t)length;


/*	if(bytes_left<0)
		bytes_left =  -EFAULT;

	return bytes_left;*/
}
terminal output:
5470.497646] @membuff_init Major number for mmap: 250
[ 5474.748628] Inside read
[ 5474.748646] @membuff_init len: 1
[ 5474.748658] @membuff_init offset: 0
[ 5474.748670] @membuff_init input_msg[0]: 0

output from application:
no:0 0
no:1 0
no:2 0
no:3 0
no:4 0
no:5 0
no:6 0
no:7 0
no:8 0
no:9 0
no:10 0
....
 
Old 02-15-2012, 08:46 AM   #6
dennisdd
Member
 
Registered: Oct 2011
Posts: 37

Original Poster
Rep: Reputation: Disabled
nominal animal!

thanks again! you catch my bugs! it works now.

I am not very comfortable with casting pointers! well pointer too

char *buffer; // char pointer

learnt something today:
sizeof (buffer) == sizeof (char *)

buffer = malloc(bufferlen); // I tried this too
read(fd, buffer, sizeof(buffer)); //then this one, so it is still the same 4 bytes only it is seeing char

buffer = (char *)malloc(bufferlen); ===(equal)==== buffer = malloc(bufferlen); // is this the same meaning?

from my current understanding "char *buffer;" actually indicates that the location memory refers are in 4 bytes.
Same goes (char *)malloc(bufferlen) also make the memory referencing to be in 4bytes.

1st 4bytes, 2nd 4bytes 3rd 4bytes....and so on.
 
Old 02-15-2012, 09:02 AM   #7
Nominal Animal
Senior Member
 
Registered: Dec 2010
Location: Finland
Distribution: Xubuntu, CentOS, LFS
Posts: 1,723
Blog Entries: 3

Rep: Reputation: 948Reputation: 948Reputation: 948Reputation: 948Reputation: 948Reputation: 948Reputation: 948Reputation: 948
No, I meant userspace code similar to
Code:
    size_t   bufferlen;
    char    *buffer;
    ssize_t  bytes;

    /* Decide you want 25 ints. */
    bufferlen = 25 * sizeof(int);

    /* Allocate buffer. */
    buffer = malloc(bufferlen);
    if (!buffer) {
        /* Out of memory, abort */
    }

    /* Read buffer full of data. */
    bytes = read(descriptor, buffer, bufferlen);
    if (bytes > (ssize_t)0) {
        /* Okay, got (bytes / sizeof(int)) ints in buffer,
         * int k being ((int *)buffer)[k] */
    }
Or, since you work on ints, perhaps
Code:
    int      int_count = 5;  /* Do 5 ints. */
    int     *buffer;
    int      i, got_ints;
    ssize_t  n;

    buffer = malloc(sizeof(int) * (size_t)int_count);
    if (!buffer) {
        fprintf(stderr, "Out of memory.\n");
        exit(1);
    }

    n = read(descriptor, buffer, sizeof(int) * (size_t)int_count);
    if (n > (ssize_t)0) {
        got_ints = n / sizeof(int);
        for (i = 0; i < got_ints; i++)
            printf("Int %d = %d\n", i, buffer[i]);
    }
As to the malloc() function, since it returns an untyped pointer (void *, also called void pointer), you do not need to cast the result. No matter what kind of a pointer buffer is (char * or int * or anything else), you can always do
Code:
buffer = malloc(number_of_bytes_to_allocate);
Since the count is in bytes, and sizeof(char)==1 , to allocate say 47 time_t values you need to do
Code:
buffer = malloc(47 * sizeof(time_t));
The malloc() does not care what kind of pointer you use to store the result to, it only looks at its own parameter (for the number of bytes/chars to allocate).
 
Old 04-12-2012, 04:15 PM   #8
snn
LQ Newbie
 
Registered: Apr 2012
Posts: 1

Rep: Reputation: Disabled
mini6410 adc

Hi everyone I am working on educational project on mini6410 device.I have to access ADC port and I have and driver source code as team3adc.c below ,but at the end of compile this source code node didn't opened under /dev .Please ,help me!!!! This is very urgent.

#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/fs.h>
#include <linux/cdev.h>
#include <linux/types.h>
#include <linux/slab.h>
#include <linux/ioport.h>
#include <linux/spinlock.h>
#include <linux/errno.h>
#include <linux/delay.h>
#include <linux/interrupt.h>
#include <linux/irq.h>

#include <asm/io.h>
#include <asm/delay.h>
#include <asm/uaccess.h>
#include <asm/arch/irqs.h>
#include <asm-arm/arch-s3c2410/smdk2410.h>

#include "map.h"
#include "regs-adc.h"
#include "regs-clock.h"

#ifndef ADC_MAJOR
#define ADC_MAJOR (243)
#endif

#ifndef DEVICENAME
#define DEVICENAME ("team3adc")
#endif

#define CLK2410 (50*1000*1000)
#define ADCFREQ (2500000)

#define ADCCON (S3C24XX_VA_ADC+S3C2410_ADCCON)
#define ADCDAT0 (S3C24XX_VA_ADC+S3C2410_ADCDAT0)


static void adc_disable(void)
{
writew(0x3fc4,ADCCON);
}

static int adc_enable(int cn)
{
int prescaler=CLK2410/ADCFREQ-1;/*pclk=66mhz,adc_freq=2500000 */
u16 x;

printk(KERN_ALERT "prescaler is %d\r\n",prescaler);

x=(1<<14)|(5<<6)|(cn<<3);

writew(x,ADCCON);
//printk(KERN_ALERT "adccon is %x\r\n",readw(ADCCON));

msleep(50);
return 0;

}

int fops_release(struct inode *inode,struct file *filp)
{
adc_disable();
return 0;
}

int fops_open(struct inode *inode , struct file *filp)
{

u16 clock;
clock=readw(S3C2410_CLKCON);
clock=clock | S3C2410_CLKCON_ADC;
writew(clock,S3C2410_CLKCON);

adc_enable(0);

return 0;
}

static ssize_t fops_read(struct file *filp ,char __user *buff,
size_t count,loff_t *offp)
{
u16 kbuf[1];
u16 x;
adc_enable(0);

x=readw(ADCCON);
x=x | 1;
writew(x,ADCCON);

//x=readw(ADCCON);

while(readw(ADCCON) & 0x1);

msleep(50);

x=readw(ADCDAT0);
kbuf[0]=x;

printk(KERN_ALERT "adcdat0 is %x\r\n",readw(ADCDAT0));

copy_to_user(buff, kbuf, count);

return 2;
}

int fops_ioctl(struct inode *inode,struct file *filp,
unsigned int cmd,unsigned long arg)
{
return 0;
}


struct file_operations adc_fops =
{
.owner=THIS_MODULE,
.read=fops_read,
.open=fops_open,
.release=fops_release,
.ioctl=fops_ioctl,
};

static void __exit adc_cleanup (void)
{
printk("module exit\n");
unregister_chrdev(ADC_MAJOR,DEVICENAME);
}

static int __init adc_init(void)
{
int result;

printk("adc init\n");
result=register_chrdev(ADC_MAJOR,DEVICENAME,&adc_fops);
if(result<0)
{
printk("adc:can't get major %d\n",ADC_MAJOR);
return result;
}
return 0;
}


MODULE_AUTHOR ("akae-wjy");
MODULE_LICENSE ("GPL");

module_init (adc_init);
module_exit (adc_cleanup);

Last edited by snn; 04-12-2012 at 04:21 PM.
 
  


Reply

Tags
linux module


Thread Tools Search this Thread
Search this Thread:

Advanced Search

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is Off
HTML code is Off



Similar Threads
Thread Thread Starter Forum Replies Last Post
Free user space pages of different user processes from inside kernel space trueskyte Linux - Kernel 1 10-22-2010 04:37 PM

LinuxQuestions.org > Forums > Non-*NIX Forums > Programming

All times are GMT -5. The time now is 06:29 PM.

Main Menu
Advertisement
My LQ
Write for LQ
LinuxQuestions.org is looking for people interested in writing Editorials, Articles, Reviews, and more. If you'd like to contribute content, let us know.
Main Menu
Syndicate
RSS1  Latest Threads
RSS1  LQ News
Twitter: @linuxquestions
Open Source Consulting | Domain Registration