Unable to spot the error in echo kernel module

Question

I am trying to write a very simple echo Linux driver.

The driver takes a maximum of 250 characters from command-line and just writes it into a dummy device 'mydev'. This is again read back from the device. The front end and driver code is pasted below for reference.

The issue is I am able to write but not read. There is no error on compilation or segmentation fault. But none of the messages in printk in the driver's read are printed. I am puzzled as to what is happening. Can I get some clue here?

I am just sharing the code copy for better clarity:

mydriver.c :

    #include <linux/module.h>
    #include <linux/fs.h>
    #include <asm/uaccess.h>

    MODULE_LICENSE("GPL");

    static int major;
    static char kbuf[250];

    static int dv_open(struct inode *inode, struct file *filp)
    {
        return 0; 
    }

    static int dv_close(struct inode *inode, struct file *filp)
    {
        return 0; 
    }

    static ssize_t dv_read(struct file *filp, char __user *buf, 
                    size_t sz, loff_t *fpos)
    {
        int r;
        int L;
        printk("READ:Entering\n");

        L = strlen(kbuf);
        r = copy_to_user(buf, kbuf, L);

        printk("READ:Ends\n");

        return L; 
    }
    static ssize_t dv_write(struct file *filp, const char __user *buf, 
                    size_t sz, loff_t *fpos)
    {
        int r, wr_sz;

        printk("WRITE:Entering\n");
        memset(kbuf,'\0', 250);
        if ( sz <= 250 ) {
           wr_sz = sz;
        } else {
           wr_sz = 250;
        }
        r = copy_from_user(kbuf, buf, wr_sz);

        printk("WRITE:Rx buf = %s\n", kbuf);

        return 0;

    }
    static struct file_operations dv_fops = {
        .open     = dv_open,
        .release  = dv_close,
        .read     = dv_read,
        .write    = dv_write,
        .owner    = THIS_MODULE,
    };

    int init_module(void)
    {
        major = register_chrdev(0, "dvdev", &dv_fops);
        if ( major < 0 ) {
           printk("Error in registering driver\n");
           return -1;
        }
        else printk("Success. major = %d\n", major);
        return 0;
    }

    void cleanup_module(void)
    {
        unregister_chrdev(major, "dvdev");
    }

myuserapp.c

    #include <stdio.h>
    #include <fcntl.h>
    #include <string.h>

    static char buf[250];
    static char * wbuf;
    int main(int argc, char **argv)
    {
        int fd;
        int option;
        int nbr = 0, len;

        if ( argc != 2 ) {
           printf("usage: front <devName>\n");
           return -1;
        }
        fd = open("mydev", O_RDONLY | O_WRONLY);
        if ( fd < 0 ) {
           printf("Error opening file. %s does not exist\n", argv[1]);
           return -2;
        }

        wbuf = argv[1];
        len = strlen(wbuf);
        nbr = write(fd, wbuf, len); 
        printf("USR: Buf written = %s, nbr = %d\n", wbuf ,nbr);


        nbr = read(fd, buf, 250);
        printf("USR RD: %s", buf);

        close(fd);
        return 0;
    }

Answer 1

Each calls to your write function will result in calling memset on the buffer.This is the root cause you are not able to retrieve the message.

Answer 2

sawdust gave the right answer, but there's another problem in your code.

If you write 250 bytes to your device, the buffer is not null-terminated. Then, when reading, strlen will read beyond it, leading to an unexpected result.

Answer 3

Your code has at least one error:

 fd = open("mydev", O_RDONLY | O_WRONLY);

That is an improper open() call.
The man page for open() specifies that:

Applications shall specify exactly one of the first three values (file access modes) below in the value of oflag:

O_RDONLY Open for reading only.
O_WRONLY Open for writing only.
O_RDWR Open for reading and writing. The result is undefined if this flag is applied to a FIFO.

Instead of specifying only one, you have an expression of two values.

I guess O_RDONLY and O_WRONLY are bits individually, so it would be something like O_RDONLY | O_WRONLY = 10|01 = 11 . Both the bits of Read and write are set.

The bit values are irrelevant, since combining these values is not allowed.
You seem to be ignoring the exclusionary suffix "ONLY".
RDONLY means "allow read and disallow write".
WRONLY means "allow write and disallow read".
"O_RDONLY | O_WRONLY" is a logical contradiction.
If you want to allow both reading and writing, then you have to specify O_RDWR.

And Mark Stevens has provided the correct values and Boolean arithmetic to prove that your inproper expression is not equivalent to O_RDWR.