0

I am writing a LKM to create a character device driver.

Linux Kernel: 4.4.0-93-generic in VirtualBox, 2GB ram and SWAP is 300Kb

Problem 1

If I write a C program that handles the fd in dev_write, it's all good, it reads as it should, but if I try to use head -n 1 < /dev/opsysmem it does not output anything.

Code for reading from device:

int main()
{
    int ret, fd;
    char stringToSend[BUFFER_LENGTH];
    printf("Starting device test code example...\n");
    fd = open("/dev/opsysmem", O_RDWR); // Open the device with read/write access
    if (fd < 0)
    {
        perror("Failed to open the device...");
        return errno;
    }

    printf("Press ENTER to read back from the device...\n");
    getchar();

    printf("Reading from the device...\n");
    ret = read(fd, receive, BUFFER_LENGTH); // Read the response from the LKM
    if (ret < 0)
    {
        perror("Failed to read the message from the device.");
        return errno;
    }
    printf("The received message is: [%s]\n", receive);

    return 0;
}

Problem 2

If I repeatedly send a message big enough, everything is fine, my 2MiB buffer fills up and then the following messages are discarded. However, if the message is smaller (i.e 1 char each), it stops after about 10000 nodes. Is this a problem with my implementation of the linked list, a known linux problem or it's just me not observing something in my code?

When I encounter Problem 2, the vCPU throttles in sinusoidal manner

Here are my functions for read, write:

static ssize_t dev_read(struct file *filep, char *buffer, size_t len, loff_t *offset) {
    Node *msg;
    int ret_val;
    char* message;
    unsigned int message_length;

    // Entering critical section
    down(&sem); //wait state

    msg = pop(&l, 0);

    // No message? No wait!
    if(!msg) {
        up(&sem);
        return -EAGAIN;
    }

    if(len < msg->length) {
        up(&sem);
        return -EINVAL;
    }

    // Since we have a message, let's send it!
    current_size -= message_length;

    // copy_to_user has the format ( * to, *from, size) and returns 0 on success
    ret_val = copy_to_user(buffer, msg->string, message_length);

    if (!ret_val) {
        remove_element(&l, 0);
        up(&sem);
        return ret_val;
    } else {
        up(&sem);
        return -EFAULT; // Failed
    }
}


static ssize_t dev_write(struct file *filep, const char *buffer, size_t len, loff_t *offset) {
    Node *n;

    // buffer larger than 2 * 1024 bytes
    if(len > MAX_MESSAGE_SIZE || len == 0) {
        return -EINVAL;
    }

    n = kmalloc(sizeof(Node), GFP_KERNEL);

    if(!n) { 
        return -EAGAIN;
    }

    n->string = (char*) kmalloc(len, GFP_KERNEL);
    n->length = len;

    copy_from_user(n->string, buffer, len);

    // Enter critical section
    down(&sem); //wait state
    if(SLEEP) msleep(100);

    // buffer is larger than the total list memory (2MiB)
    if(current_size + len > MAX_LIST_SIZE) {
        up(&sem);
        return -EAGAIN;
    }

    current_size += len;

    push(&l, n);

    up(&sem);
    // Exit critical section

    return len;
}

And here is my linked list and its functions

typedef struct Node {
    unsigned int length;
    char* string;
    struct Node *next;
} Node;

typedef struct list{
    struct Node *node;
} list;

static void init(list * l){
    l->node = NULL;
}

static void destroyNode(Node *n) {
    if(n) {
        destroyNode(n->next);
        kfree(n->string);
        n->string = NULL;
        kfree(n);
        n = NULL;
    }
}

static void destroy(list *l){
    if(l) {
        destroyNode(l->node);
    }
}

static Node* pop(list *l, unsigned int index) {
    struct Node *_current = l->node;

    // Cut down index until reaching the desired position
    while(_current) {
        if(index) {
            _current = _current->next;
            index--;
        } else { return _current; }
    }

    // If you are here, the node does not exist
    return NULL;
}

static int push(list * l, Node *n) {
    if(!n) { return -1; }

    // Initialize the string

    // Do we have a node in the list?
    if (l->node) { 
        // Virtually add it as a head
        n->next = l->node;
    } else {
        n->next = NULL;
    } // Otherwise prepare the list to have no tail

    // Now make the list point to the head
    l->node = n;

    return 0;
}

static int remove_element(list * l, unsigned int index){
    // Get the reference for head
    struct Node *previous;
    struct Node *_current;

    previous = NULL;
    _current = (Node*) l->node;

    // Swap _current until index
    while(_current) {
        // Is the index !0 and we have more nodes?
        if(index) {
            previous = _current;
            _current = _current->next;
            index--;
        } else {
            if(previous) {
                previous->next = _current->next;
            } else {
                l->node = _current->next;
            }
            // Free memory, assign NULL pointer
            kfree(_current->string);
            _current-> string = NULL;
            kfree(_current);
            _current = NULL;

            // Return success
            return 0;
        }
    }

    // No _current? No problem!
    return -1;
}

Update on __ Problem 2 __ I tried different sizes for the input string and I found this: After about 650 calls to the device driver, with a size of 3.3k, the message list size becomes 4MiB (which is the maximum). A few more calls are made to the device and then the kernel freezes.

EDIT 1: I updated te dev_write as per comment and deleted debug code EDIT 2: Added some more functions: push/pop/destroy EDIT 3: I put in the check for buffer length vs message length

  • 2
    "If I write a C program that handles the fd in dev_write" ?? this does not mean much to me ... Also, head will (should) call dev_read (head reads from the file). And more, the kernel provides a working and well tested linked list which you can find in linux/list.h in kernel sources, do not implement another one, use existing one instead. – Nathael Pajani Nov 21 '19 at 1:15
  • @NathaelPajani, I put the code for my reader from device. I mean this outputs the message that dev_read copies from kernel space to user space. head -n 1 < /dev/opensysmem does not. dev_read is called (the message is taken out from the list, but nothing is printed to the user. (PS I tried adding a newline in the message passed to the kernel and read it, but still nothing) – bem22 Nov 21 '19 at 8:42
  • 1
    dev_write() must do a copy_from_user() before doing the push() - you cannot pass the buffer/len received from dev_write() to copy_to_user() in dev_read() - that data is still in user space ... (read this); other comments: in dev_read(), you don't check that message_length is <= len; also, in both dev_read() and dev_write() you ignore the offset argument - maybe the "head" program uses seek for some things? also "head" will need to see an end-of-line char (i.e. newline). – Murray Jensen Nov 21 '19 at 14:01
  • @MurrayJensen I would accept that as an answer if you do a write-up. I was totally new to copying memory from kernel space to user space and vice-versa. Thanks for that. – bem22 Nov 22 '19 at 1:45
2

I think Problem 1 may be either because head is not seeing an end-of-line character (e.g. newline,'\n'), or it uses seek system calls and you ignore the offset argument in your dev_read() and dev_write() functions (which means seeks won't work, if I understand it correctly) ... check this out - head does try to optimise things using seeks, but not sure if it applies in your case.

Also not sure your answer about Problem 2 being caused by time being out of sync is correct (unless it has something to do with msleep()) ... my guess would be memory allocation problems or a race condition but you don't show us the source code for push() and pop() so we can't tell.

It looks like you just store the buffer and len arguments from dev_write() and then use them in dev_read() to pass to copy_to_user() ... the data in that buffer will still be in user space, so you could be attempting to copy from user space to user space. Reading this might help.

You should update your question with the code of push() and pop() ... but at a minimum push() will need to allocate memory for both a linked list element to be inserted in the list and for a buffer to hold the write data, then use copy_from_user() to get the data out of user space and into the kernel buffer ... and then, after you have finished with msg in dev_read(), you will need to free the kernel buffer contained in msg and then free msg itself.

A lot of copying going on here I know, but to avoid this you have to work very hard with the virtual memory system and the design of your code (i.e. a Zero Copy implementation).

One more small but very important thing, in dev_read() you don't check that message_length is <= len i.e. that there is enough space for the message in the buffer. For example, as your code stands, your driver potentially could try to copy a message bigger than the space available. copy_to_user() should catch this, but then again maybe this is the source of your Problem 2.

  • I created a separate thread for the problem of deallocating the memory. I think it's a whole different issue. unix.stackexchange.com/questions/553500/… – bem22 Nov 22 '19 at 2:50
  • Also, in my implementation, I do exactly what you said. Take a look at the other issue. – bem22 Nov 22 '19 at 2:51
  • I implemented a lot of fixes from your text and I found the issue with Problem 2. Problem 1 has now evolved into a new topic. I will mark this as solved. – bem22 Nov 22 '19 at 4:12
0

I solved Problem 2. It was occurring because virtualbox was getting out of sync with the host machine.

Found the solution here https://stackoverflow.com/questions/5308492/virtualbox-synchronisation-problems

It is:

VBoxManage guestproperty set "/VirtualBox/GuestAdd/VBoxService/--timesync-set-threshold" 1000

0

Problem 1

I searched my commit history and I found a commit message saying "Working test.sh" which means it was passing the write with echo and reading with head -n 1

In this commit, my dev_read function is returning the message length.

This seems to work for both head -n 1 and cat and buffered read from the mentioned C program.

I found the answer here: https://www.tldp.org/LDP/lkmpg/2.4/html/c577.htm

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.