10 replies to this topic

[onus]

I wrote a small hello world type of character device driver.
When I type echo -n "abcdef" > /dev/bond
and do a cat /dev/bond
then only last "f" of above input abcdef is displayed rest nothing is displayed.
I have done and experimented many things but I am unable to catch the error.

Can some one point out the error?
Here is the code

/* Necessary includes for device drivers */

#include <linux/init.h>

//#include <linux/config.h>

#include <linux/module.h>

#include <linux/kernel.h> /* printk() */

#include <linux/slab.h> /* kmalloc() */

#include <linux/fs.h> /* everything... */

#include <linux/errno.h> /* error codes */

#include <linux/types.h> /* size_t */

#include <linux/proc_fs.h>

#include <linux/fcntl.h> /* O_ACCMODE */

#include <asm/system.h> /* cli(), *_flags */

#include <asm/uaccess.h> /* copy_from/to_user */


MODULE_LICENSE("Dual BSD/GPL");


/* Declaration of memory.c functions */

int memory_open(struct inode *inode, struct file *filp);

int memory_release(struct inode *inode, struct file *filp);

ssize_t memory_read(struct file *filp, char *buf, size_t count, loff_t *f_pos);

ssize_t memory_write(struct file *filp, char *buf, size_t count, loff_t *f_pos);

void memory_exit(void);

int memory_init(void);


/* Structure that declares the usual file */

/* access functions */

struct file_operations memory_fops = {

  read: memory_read,

  write: memory_write,

  open: memory_open,

  release: memory_release

};

/* Declaration of the init and exit functions */

module_init(memory_init);

module_exit(memory_exit);


/* Global variables of the driver */

/* Major number */

int memory_major = 60;

/* Buffer to store data */

char *memory_buffer;


int memory_init(void) {

  int result;


  /* Registering device */

  result = register_chrdev(memory_major, "bond", &memory_fops);

  if (result < 0) {

    printk(KERN_ALERT  "memory: cannot obtain major number %d\n", memory_major);

    return result;

  }


  /* Allocating memory for the buffer */

  memory_buffer = kmalloc(1, GFP_KERNEL); 

  if (!memory_buffer) { 

    result = -ENOMEM;

    goto fail; 

  } 

  memset(memory_buffer, 0, 10);


  printk(KERN_ALERT "Inserting bond module\n"); 

  return 0;


  fail: 

    memory_exit(); 

    return result;

}



void memory_exit(void) {

  /* Freeing the major number */

  unregister_chrdev(memory_major, "bond");


  /* Freeing buffer memory */

  if (memory_buffer) {

    kfree(memory_buffer);

  }


  printk( KERN_ALERT "Removing bond module\n");


}



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


  /* Success */

  return 0;

}



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

 

  /* Success */

  return 0;

}



ssize_t memory_read(struct file *filp, char *buf, 

                    size_t count, loff_t *f_pos) { 

 

  /* Transfering data to user space */ 

  copy_to_user(buf,memory_buffer,10);


  /* Changing reading position as best suits */ 

  if (*f_pos == 0) { 

    *f_pos+=1; 

    return 1; 

  } else { 

    return 0; 

  }

}


ssize_t memory_write( struct file *filp, char *buf,

                      size_t count, loff_t *f_pos) {


  char *tmp;


  tmp=buf+count-1;

  copy_from_user(memory_buffer,tmp,10);

  return 1;

}

[dbug]

When you allocate memory, you should specify at least 10 bytes in kmalloc.

Why are you doing this in memory_write ?

tmp = buf + count - 1;

copy_from_user(memory_buffer, tmp, 10);

Shouldn't it be this ?

copy_from_user(memory_buffer, buf, count < 10 ? count : 10);

In memory_read this would be better:

copy_to_user(buf, memory_buffer, count < 10 ? count : 10);

In both functions you should return the number of bytes read/write (that is count or 10 if count is greater).

[onus]

I did what you suggested this time there was no output

/* Necessary includes for device drivers */
#include <linux/init.h>
//#include <linux/config.h>
#include <linux/module.h>
#include <linux/kernel.h> /* printk() */
#include <linux/slab.h> /* kmalloc() */
#include <linux/fs.h> /* everything... */
#include <linux/errno.h> /* error codes */
#include <linux/types.h> /* size_t */
#include <linux/proc_fs.h>
#include <linux/fcntl.h> /* O_ACCMODE */
#include <asm/system.h> /* cli(), *_flags */
#include <asm/uaccess.h> /* copy_from/to_user */

MODULE_LICENSE("Dual BSD/GPL");

/* Declaration of memory.c functions */
int memory_open(struct inode *inode, struct file *filp);
int memory_release(struct inode *inode, struct file *filp);
ssize_t memory_read(struct file *filp, char *buf, size_t count, loff_t *f_pos);
ssize_t memory_write(struct file *filp, char *buf, size_t count, loff_t *f_pos);
void memory_exit(void);
int memory_init(void);

/* Structure that declares the usual file */
/* access functions */
struct file_operations memory_fops = {
  read: memory_read,
  write: memory_write,
  open: memory_open,
  release: memory_release
};
/* Declaration of the init and exit functions */
module_init(memory_init);
module_exit(memory_exit);

/* Global variables of the driver */
/* Major number */
int memory_major = 60;
/* Buffer to store data */
char *memory_buffer;

int memory_init(void) {
  int result;

  /* Registering device */
  result = register_chrdev(memory_major, "bond", &memory_fops);
  if (result < 0) {
    printk(KERN_ALERT  "memory: cannot obtain major number %d\n", memory_major);
    return result;
  }

  /* Allocating memory for the buffer */
  memory_buffer = kmalloc(1, GFP_KERNEL); 
  if (!memory_buffer) { 
    result = -ENOMEM;
    goto fail; 
  } 
  memset(memory_buffer, 0, 10);

  printk(KERN_ALERT "Inserting bond module\n"); 
  return 0;

  fail: 
    memory_exit(); 
    return result;
}


void memory_exit(void) {
  /* Freeing the major number */
  unregister_chrdev(memory_major, "bond");

  /* Freeing buffer memory */
  if (memory_buffer) {
    kfree(memory_buffer);
  }

  printk( KERN_ALERT "Removing bond module\n");

}


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

  /* Success */
  return 0;
}


int memory_release(struct inode *inode, struct file *filp) {
 
  /* Success */
  return 0;
}


ssize_t memory_read(struct file *filp, char *buf, 
                    size_t count, loff_t *f_pos) { 
 
  /* Transfering data to user space */ 
  copy_to_user(buf,memory_buffer,count<10 ? count:10);

  /* Changing reading position as best suits */ 
  if (*f_pos == 0) { 
    *f_pos+=1; 
    return 1; 
  } else { 
    return 0; 
  }
}

ssize_t memory_write( struct file *filp, char *buf,
                      size_t count, loff_t *f_pos) {

  char *tmp;

  //tmp=buf+count-1;
  copy_from_user(memory_buffer,tmp,count<10 ? count : 10);
  return 1;
}

doing

echo -n "abcde" > /dev/bond

and then

cat /dev/bond

did not gave me any output.

[dbug]

If you don't initialize tmp in memory_write, you can't use it in copy_from_user. You must replace tmp with buf (it's nice you haven't get a kernel panic...)

kmalloc needs to alloc at least 10 bytes. You must also change this.

I'm not sure if it's strictly necessary, but I think you should be consequent with the number of bytes processed and the value of f_pos and the returned size in memory_read and memory_write functions.

[onus]

Ok I have modified the code a bit
here is what I have to post if you want to try install kernel headers

#include <linux/init.h>

#include <linux/module.h>
#include <linux/kernel.h> /* printk() */
#include <linux/slab.h> /* kmalloc() */
#include <linux/fs.h> /* everything... */
#include <linux/errno.h> /* error codes */
#include <linux/types.h> /* size_t */
#include <linux/proc_fs.h>
#include <linux/fcntl.h> /* O_ACCMODE */
#include <asm/system.h> /* cli(), *_flags */
#include <asm/uaccess.h> /* copy_from/to_user */

MODULE_LICENSE("Dual BSD/GPL");


int bond_open(struct inode *inode, struct file *filp);
int bond_release(struct inode *inode, struct file *filp);
ssize_t bond_read(struct file *filp, char *buf, size_t count, loff_t *f_pos);
ssize_t bond_write(struct file *filp, char *buf, size_t count, loff_t *f_pos);
void bond_exit(void);
int bond_init(void);

struct file_operations bond_fops = {
  read: bond_read,
  write: bond_write,
  open: bond_open,
  release: bond_release
};

module_init(bond_init);
module_exit(bond_exit);

int bond_major = 60;

char *bond_buffer;

int bond_init(void) {
  int result;


  result = register_chrdev(bond_major, "bond", &bond_fops);
  if (result < 0) {
    printk(KERN_ALERT  "memory: cannot obtain major number %d\n", bond_major);
    return result;
  }


  bond_buffer = kmalloc(14, GFP_KERNEL); 
  if (!bond_buffer) { 
    result = -ENOMEM;
    goto fail; 
  } 
  memset(bond_buffer, 0, 14);

  printk(KERN_ALERT "Inserting bond module\n"); 
  return 0;

  fail: 
    bond_exit(); 
    return result;
}


void bond_exit(void) {

  unregister_chrdev(bond_major, "bond");


  if (bond_buffer) {
    kfree(bond_buffer);
  }

  printk( KERN_ALERT "Removing bond module\n");

}


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


  return 0;
}


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

  return 0;
}


ssize_t bond_read(struct file *filp, char *buf, 
                    size_t count, loff_t *f_pos) { 
 

  copy_to_user(buf,bond_buffer,count<14 ? count:14);


 /* if (*f_pos == 0) { 
    *f_pos+=1; 
    return 1; 
  } else { 
    return 0; 
  }*/
}

ssize_t bond_write( struct file *filp, char *buf,
                      size_t count, loff_t *f_pos) {

//  char *tmp;

  //tmp=buf+count-1;
  copy_from_user(bond_buffer,buf,count<14 ? count : 14);
  return 1;
}

here is the Makefile

ifeq ($(KERNELRELEASE),)
KERNELDIR ?= /lib/modules/$(shell uname -r)/build
PWD := $(shell pwd)
.PHONY: build clean
build:
	$(MAKE) -C $(KERNELDIR) M=$(PWD) modules
clean:
	rm -rf *.o *~ core .depend .*.cmd *.ko *.mod.c
	rm -rf modules.order Module.symvers
else
$(info Building with KERNELRELEASE =${KERNELRELEASE})
obj-m := bond.o

endif

[dbug]

Does it work now ?

[onus]

No same problem it did not worked.It is assigning only last character of the string that I passed it on.
Here is the original tutorial from where I am trying it
Writing device drivers in Linux: A brief tutorial
here is the strace

mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f2a2f03b000
write(1, "abcde", 5)                    = 1
write(1, "bcde", 4)                     = 1
write(1, "cde", 3)                      = 1
write(1, "de", 2)                       = 1
write(1, "e", 1)                        = 1
close(1)                                = 0
munmap(0x7f2a2f03b000, 4096)            = 0
close(2)                                = 0
exit_group(0)                           = ?

so copy_from_user and copy_to_write are not doing what I expect them to do this is not clear to me.
I got a message from some one they were not able to debug it but they pointed me some hint I am pasting if you can understand the following hint I also was clear but not the error on my driver.

Quote

User mode tried to write 5 characters.
The driver only accepted one (which is evidenced by the return code
which is the number of characters which the driver accepted).

>>> write(1, "bcde", 4) = 1

So usermode then tried to write the remaining 4 characters.
The driver only accepted one.

>>> write(1, "cde", 3) = 1

[onus]

Ok here is a code which is actually working I am sharing it so that if some one comes across this thread should help them.

/* Necessary includes for device drivers */
#include <linux/init.h>
//#include <linux/config.h>
#include <linux/module.h>
#include <linux/kernel.h> /* printk() */
#include <linux/slab.h> /* kmalloc() */
#include <linux/fs.h> /* everything... */
#include <linux/errno.h> /* error codes */
#include <linux/types.h> /* size_t */
#include <linux/proc_fs.h>
#include <linux/fcntl.h> /* O_ACCMODE */
#include <asm/system.h> /* cli(), *_flags */
#include <asm/uaccess.h> /* copy_from/to_user */

MODULE_LICENSE("Dual BSD/GPL");

/* Declaration of memory.c functions */
int memory_open(struct inode *inode, struct file *filp);
int memory_release(struct inode *inode, struct file *filp);
ssize_t memory_read(struct file *filp, char *buf, size_t count, loff_t *f_pos);
ssize_t memory_write(struct file *filp, const char __user *buf, size_t count, loff_t *f_pos);
void memory_exit(void);
int memory_init(void);

/* Structure that declares the usual file */
/* access functions */
struct file_operations memory_fops = {
	read: memory_read,
   	write: memory_write,
	open: memory_open,
	release: memory_release
};
/* Declaration of the init and exit functions */
module_init(memory_init);
module_exit(memory_exit);

/* Buffer to store data */
char *memory_buffer;
int result;

int memory_init(void) {

	/* Registering device */
	result = register_chrdev(0, "bond", &memory_fops);
	if (result < 0) {
		printk(KERN_ALERT  "memory: cannot obtain major number \n");
		return result;
	}

	/* Allocating memory for the buffer */
	memory_buffer = kmalloc(10, GFP_KERNEL); 
	if (!memory_buffer) { 
		result = -ENOMEM;
		goto fail; 
	} 
	memset(memory_buffer, 0, 10);

	printk(KERN_ALERT "Inserting bond module\n"); 
	return 0;

fail: 
	memory_exit(); 
	return result;
}


void memory_exit(void) {
	/* Freeing the major number */
	unregister_chrdev(result, "bond");

	/* Freeing buffer memory */
	if (memory_buffer) {
		kfree(memory_buffer);
	}

	printk( KERN_ALERT "Removing bond module\n");

}


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

	/* Success */
	return 0;
}


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

	/* Success */
	return 0;
}


ssize_t memory_read(struct file *filp, char __user *buf, 
		size_t count, loff_t *f_pos) 
{
	if (*f_pos > 0)
		return 0;
	if (count > strlen(memory_buffer))
		count = strlen(memory_buffer);
	copy_to_user(buf,memory_buffer,count);
	*f_pos = *f_pos + count;

	return count; 
}

ssize_t memory_write( struct file *filp, const char __user *buf,
		size_t count, loff_t *f_pos) 
{
	copy_from_user(memory_buffer, buf, count);
	return count;
}

is the one which actually worked.
The reason for this is

Quote

The first read comes in with a *f_pos of 0. If you dont update the file pointer, the user space program will keep reading from f_pos 0 indefinitely. As file pointer is at offset 0 always because you dont update *f_pos, every read(..) called by user space will succeed. So, your command cat /dev/bond will never return. user space programs generally look for a specific return code to know that it is the end of the file.

That is the reason in strace I was getting

Quote

mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f2a2f03b000

write(1, "abcde", 5) = 1
write(1, "bcde", 4) = 1
write(1, "cde", 3) = 1
write(1, "de", 2) = 1

write(1, "e", 1) = 1
close(1) = 0
munmap(0x7f2a2f03b000, 4096) = 0
close(2) = 0
exit_group(0) = ?

[dbug]

As I told you before, you should be consequent with the number of bytes processed in read and write functions. This is a possible solution:

ssize_t bond_read(struct file *filp, char *buf, size_t count, loff_t *f_pos)

{

    if (*f_pos >= 14)

    {

        return 0;

    }

    size_t size = 14 - *f_pos;

    if (count < size)

    {

        size = count;

    }

    copy_to_user(buf, bond_buffer + *f_pos, size);

    (*f_pos) += size;


    return size;

}


ssize_t bond_write(struct file *filp, char *buf, size_t count, loff_t *f_pos)

{

    if (*f_pos < 14)

    {

        size_t size = 14 - *f_pos;

        if (count < size)

        {

            size = count;

        }

        copy_from_user(bond_buffer + *f_pos, buf, size);

        (*f_pos) += size;

    }


    return count;

}

On write, it will accept any number of bytes, but only first 14 will be recorder. On read, it will send 14 bytes (even if the previous write recorded less than 14 bytes).

There are some potential issues with this driver because you allow more than one writer to be recording data simultaneously.

I would also create a constant to define the size of the buffer to be recorded. It's easier to change it later:

#define BUFF_SIZE 14

[dbug]

I've just seen the solution you have found. It might work, but there is a dangerous bug. All memory transfers between user space and kernel space must be strictly controlled. You must verify the size of the data to read or write before doing any transfer.

Also, your solution only works if the read and write operations are done in a single call.

[onus]

You are right actually I am learning how to write device drivers.Read a lot of books ,search forums and this that nothing worked finally some thing which I was able to make work is what I wrote above.
I understand your point about buffer overflow which I have not checked.I have not implemented any sort of mutex or semaphores in above code.
I shall be doing it this week.If you also want to understand how to write device drivers on Unix and such stuff check this link
Writing device drivers in Linux: A brief tutorial
Thanks for your help.