Dealing with buf between user space and kernel

I'm making a simple device driver capable of receiving and sending characters using a UART.

My read and write functions are as follows:

unsigned char UART_read(void){
     unsigned int buf;
     while( ( ( inb(UART_LSR + UART) ) & UART_LSR_DR ) == 0 ){
             schedule();
     }
     buf = inb(UART);
     return (char)buf;
}

ssize_t serp_read(struct file *filep, char __user *buf, size_t count, loff_t *offp){
        ssize_t cnt, ret;
        char *buffer;
        unsigned char data;
        int i;

        buffer = kmalloc(count * sizeof(char), GFP_KERNEL);

        printk("\nTHIS IS KERNEL, read was called and count is %zd\n", count);

        while(1){        
           buffer[i] = UART_read();
           if(buffer[i] == '\n') break;
           i++;
        }

        buffer[strlen(buffer) - 1] = '\0';

        if( (cnt = (copy_to_user(buf, buffer, strlen(buffer)))) != 0 )
               printk("Error in copy_to_user() cnt is %d\n", cnt);

        ret = strlen(buffer);

        printk("\nTHIS IS KERNEL, read is going away and buf is %s\n", buf);

        return ret; 
}

void UART_send(unsigned char data){
       while( ( ( inb(UART_LSR + UART) ) & UART_LSR_THRE ) == 0 ){
             schedule();
       }
       outb(data, (UART + UART_TX));
}

ssize_t serp_write (struct file *filp, const char __user *buf, size_t count, loff_t *f_pos){
        ssize_t cnt, ret;
        int i;
        char *buffer;

        buffer = kmalloc(count * sizeof(char), GFP_KERNEL);

        if( (cnt = (copy_from_user(buffer, buf, count))) != 0 ) printk("Error in     copy_from_user()\n");
        buffer[count] = '\0';

        for(i = 0; i < strlen(buffer); i++){
            UART_send(buffer[i]);
        }

        ret = strlen(buffer);

        return ret; 
}

And part of the program I'm using to test is:

    char *str = "HELLO MY NAME IS";
    strcpy(buffer, str);
    printf("\nThe message is [ %s ]\n", buffer);
    if ( (write(fd, buffer, strlen(buffer))) < 0) perror("Error");

    buffer[0] = '\0';

    if ( (read(fd, buffer, sizeof(buffer))) < 0) perror("Error");

    //buffer[strlen(buffer)] = '\0';

    printf("\nThe content of buffer after read() is [ %s ]\n", buffer);

With what I have, I have no issues writing the "HELLO, MY NAME IS" string, but when I read, let's say "hey" from the UART, the buf in the read function appears as "heyLO, MY NAME IS", and I don't understand why this overwrite is happening. My best guess right now is that first I write to user space, which the kernel accesses, and then I read to that same user space, and ends up overwriting what's already there.

Answers


read returns the number of bytes read, and doesn't nul-terminate. Further, this line:

buffer[strlen(buffer)] = '\0';

does nothing, because the strlen of buffer is the offset of the first '\0'! Try

ssize_t rb = read(fd, buffer, sizeof(buffer));
if ( rb < 0)
    perror("Error");
else {
    buffer[rb] = '\0';
    ...

Re-reading serp_read as well, I see this broken use of strlen is everywhere. If you don't have a nul-terminated C string already, you cannot use strlen. It just looks for the nul!

ssize_t serp_read(struct file *filep,
                  char __user *buf, size_t count, loff_t *offp) {
    ssize_t cnt, i;
    char *buffer;

    buffer = kmalloc(count, GFP_KERNEL);
    printk("\nTHIS IS KERNEL, read was called and count is %zd\n", count);

    /* your while loop started with i uninitialized,
     * and didn't check for overflow */
    for (i = 0; i != count; ++i) {
        buffer[i] = UART_read();
        if(buffer[i] == '\n')
            break;
    }
    /* this is incorrect because:
     * 1. strlen isn't useable unless the buffer is already nul-terminated
     * 2. the read(2) system call isn't expected to nul-terminate buffers anyway
    buffer[strlen(buffer) - 1] = '\0';
     */

    /* Use i+1 as the length if you want to include the newline */
    cnt = copy_to_user(buf, buffer, i);
    kfree(buffer);
    if(cnt) {
        printk("Error in copy_to_user() cnt is %d\n", cnt);
        i -= cnt; /* bytes successfully copied */
    }

    printk("\nTHIS IS KERNEL, read is going away and buf is %.*s\n", i, buf);
    /* this is how to print non-nul-terminated variable-length strings */

    return i; /* NB. should return an error above, as well as printing */ 
}

Use a memset, you are not clearing the memory

char *str = "HELLO MY NAME IS";

strcpy(buffer, str);
printf("\nThe message is [ %s ]\n", buffer);

if ( (write(fd, buffer, strlen(buffer))) < 0) perror("Error");

buffer[0] = '\0'; // this is clearing only single character
memset( buffer , 0 , sizeof(buffer)); // clear the entire buffer here

if ( (read(fd, buffer, sizeof(buffer))) < 0) perror("Error");

//buffer[strlen(buffer)] = '\0';

printf("\nThe content of buffer after read() is [ %s ]\n", buffer);

Need Your Help

Android emulator doesn't work properly

android android-emulator

I have installed all the necessary files from android site, but when I run my emulator it just displays "ANDROID" and nothing else. I am using Intel Dual core (2.20GHz), ASUS motherboard and 3gb o...

About UNIX Resources Network

Original, collect and organize Developers related documents, information and materials, contains jQuery, Html, CSS, MySQL, .NET, ASP.NET, SQL, objective-c, iPhone, Ruby on Rails, C, SQL Server, Ruby, Arrays, Regex, ASP.NET MVC, WPF, XML, Ajax, DataBase, and so on.