kernel driver reading ok from user space, but writing back is always 0

Question

So I'm working my way through kernel driver programming, and currently I'm trying to build a simple data transfer between application and kernel driver.

I am using simple character device as a link between these two, and I have succeeded to transfer data to driver, but I can't get meaningful data back to user space.

Kernel driver looks like this:

#include <linux/init.h>
#include <linux/module.h>
#include <linux/kernel.h> /* printk() */
#include <linux/errno.h> /* error codes */
#include <linux/types.h> /* size_t */
#include <linux/proc_fs.h>
#include <asm/uaccess.h> /* copy_from/to_user */

MODULE_LICENSE("GPL");

//Declarations
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
};

//Default 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* tx_buffer;
char* rx_buffer;

int BUFFER_SIZE=64;
int actual_rx_size=0;

int memory_init(void) {
    int result;

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

    /* Allocating memory for the buffers */
    //Allocate buffers
    tx_buffer = kmalloc(BUFFER_SIZE,  GFP_KERNEL);
    rx_buffer = kmalloc(BUFFER_SIZE,  GFP_KERNEL);

    //Check allocation was ok
    if (!tx_buffer || !rx_buffer) {
        result = -ENOMEM;
        goto fail;
    }

    //Reset the buffers
    memset(tx_buffer,0, BUFFER_SIZE);
    memset(rx_buffer,0, BUFFER_SIZE);

    printk("<1>Inserting memory module\n"); 
    return 0;

    fail:
        memory_exit(); 
        return result;
}

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

    /* Freeing buffers */
    if (tx_buffer) {
        kfree(tx_buffer); //Note kfree
    }

    if (rx_buffer) {
        kfree(rx_buffer); //Note kfree
    }
    printk("<1>Removing memory module\n");
}


//Read function
ssize_t memory_read(struct file *filp, char *buf, size_t count, loff_t *f_pos) { 

    printk("user requesting data, our buffer has (%d) \n", actual_rx_size);

    /* Transfering data to user space */ 
    int retval = copy_to_user(buf,rx_buffer,actual_rx_size);

    printk("copy_to_user returned (%d)", retval);

    return retval;
}

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

    //zero the input buffer
    memset(tx_buffer,0,BUFFER_SIZE);
    memset(rx_buffer,0,BUFFER_SIZE);

    printk("New message from userspace - count:%d\n",count);

    int retval = copy_from_user(tx_buffer,buf,count);

    printk("copy_from_user returned (%d) we read [%s]\n",retval , tx_buffer);
    printk("initialize rx buffer..\n");

    memcpy(rx_buffer,tx_buffer, count);
    printk("content of rx buffer [%s]\n", rx_buffer);

    actual_rx_size = count;

    return count; //inform that we read all (fixme?)
}

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

And the userspace application is simple as well:

#include <unistd.h>     //open, close | always first, defines compliance
#include <fcntl.h>      //O_RDONLY
#include <stdio.h>
#include <stdlib.h>     //printf
#include <string.h>

int main(int args, char *argv[])
{
int BUFFER_SIZE = 20;

char internal_buf[BUFFER_SIZE];
int to_read = 0;

memset(internal_buf,0,BUFFER_SIZE);

if (args < 3) {
    printf("2 Input arguments needed\nTo read 10 bytes: \"%s read 10\" \
    \nTo write string \"hello\": \"%s write hello\"\nExiting..\n", argv[0], argv[0]);
    return 1;
}


//Check the operation
if (strcmp(argv[1],"write") == 0) {

    printf("input lenght:%d", strlen(argv[2]));
    //Make sure our write fits to the internal buffer
    if(strlen(argv[2]) >= BUFFER_SIZE) {
        printf("too long input string, max buffer[%d]\nExiting..", BUFFER_SIZE);
        return 2;
    }

    printf("write op\n");
    memcpy(internal_buf,argv[2], strlen(argv[2]));

    printf("Writing [%s]\n", internal_buf);

    FILE * filepointer;
    filepointer = fopen("/dev/move_data", "w");
    fwrite(internal_buf, sizeof(char) , strlen(argv[2]), filepointer);
    fclose(filepointer);

} else if (strcmp(argv[1],"read") == 0) {
    printf("read op\n");

    to_read = atoi(argv[2]);

    FILE * filepointer;
    filepointer = fopen("/dev/move_data", "r");
    int retval = fread(internal_buf, sizeof(char) , to_read, filepointer);
    fclose(filepointer);

    printf("Read %d bytes from driver string[%s]\n", retval, internal_buf);
} else {
    printf("first argument has to be 'read' or 'write'\nExiting..\n");
    return 1;
}


return 0;
}

When I execute my application, this is what happens:

./rw write "testing testing"

kernel side:
[ 2696.607586] New message from userspace - count:15
[ 2696.607591] copy_from_user returned (0) we read [testing testing]
[ 2696.607593] initialize rx buffer..
[ 2696.607594] content of rx buffer [testing testing]

So all look correct. But when I try to read:

./rw read 15
read op
Read 0 bytes from driver string[]

Kernel 
[  617.096521] user requesting data, our buffer has (15) 
[  575.797668] copy_to_user returned (0)
[  617.096528] copy_to_user returned (0)

I guess it's quite simple what I'm doing wrong, since if I don't return 0, I can get some data back, but for example if I read with cat, it will continue looping endlessly.

I would like to understand what mistakes I have made in my thinking. Is there a way that kernel driver would just spit out it's buffer, and then return 0, so that I wouldn't have to build some protocol there in between to take care of how much data has been read etc.

Thanks for your suggestions!

Edit: corrected the printk statement in memory_write function, and added memory_read function trace

Answer 1

Your read function always returns 0 because you are returning retval, and not the count of bytes read. As long as the copy_to_user() call always succeeds, retval will always be 0. Instead, as long as copy_to_user() succeeds, you should return the number of bytes actually written to user space. This documentation states that copy_to_user() returns the total number of bytes that it was unable to copy.

As an aside, you are ignoring the value of count. It is very possible that the user is requesting less data than you have available in your buffer. You should never ignore count.

Now you have the problem where your function never returns a 0. Returning a 0 is important because is tells the user application that there is no more data available for reading and the user application should close the device file.

You need to keep track in your driver how many bytes have been read vs. how many bytes have been written. This may be implemented using your actual_rx_size.

Try this:

//Read function
ssize_t memory_read(struct file *filp, char *buf, size_t count, loff_t *f_pos) { 

    ssize_t bytes;

    if (actual_rx_size < count)
        bytes = actual_rx_size;
    else
        bytes = count;

    printk("user requesting data, our buffer has (%d) \n", actual_rx_size);

    /* Check to see if there is data to transfer */
    if (bytes == 0)
        return 0;

    /* Transfering data to user space */ 
    int retval = copy_to_user(buf,rx_buffer,bytes);

    if (retval) {
        printk("copy_to_user() could not copy %d bytes.\n", retval);
        return -EFAULT;
    } else {
        printk("copy_to_user() succeeded!\n");
        actual_rx_size -= bytes;
        return bytes;
    }
}