0

I am writing a kernel module. Reads bytes from user space and writes back.

static ssize_t dev_read(struct file *filep, char *buffer, size_t len, loff_t *offset) {
    Node *msg;
    int error_count = 0;

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

    msg = pop(&l, 0);

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

    len = msg->length;
    error_count = copy_to_user(buffer, msg->string, msg->length);

    if (error_count == 0) {
        current_size -= msg->length;
        remove_element(&l, 0);
        up(&sem);
        return 0;
    } else {
        up(&sem);
        printk(KERN_INFO "opsysmem: Failed to send %d characters to the user\n", error_count);
        return -EFAULT; // Failed -- return a bad address message (i.e. -14)
    }
}

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

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

Destroy function which should deallocate the linked list

static void __exit opsysmem_exit(void) {
    // Deallocate the list of messages
    down(&sem);    
    destroy(&l);
    up(&sem);
    device_destroy(opsysmemClass, MKDEV(majorNumber, 0)); // remove the device

    class_unregister(opsysmemClass);                      // unregister the device class
    class_destroy(opsysmemClass);                         // remove the device class
    unregister_chrdev(majorNumber, DEVICE_NAME);          // unregister the major number
    printk(KERN_INFO "charDeviceDriver: Goodbye from the LKM!\n");
}

My linked list and destroy function look like this:

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);
    }
}
typedef struct Node {
    unsigned int length;
    char* string;
    struct Node *next;
} Node;

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

The problem is the following:

I write to the device driver and I want to rmmod the driver and the opsysmem_exit should be called to kfree() all the memory.

This works when I have a small number of nodes.

If I run a very large amount of nodes (1000+) and I try to rmmode, the vm just freezes.

Do you have any idea why and what else I should do to diagnose this?

Is my function creating too many levels of recursion?

There does not seem to be a problem if I write 2000000 nodes and then I read them back. As far as the list is empty when I rmmod, everything works.

EDIT 1: I noticed that if I do rmmod without deallocating the memory, the kernel does not crash. However, all the memory allocated is leaked as shown by kedr

3
  • 1
    Try taking the semaphore before calling destroy... and yes, avoid recursion- just traverse the list, being careful not to access memory after freeing it – Murray Jensen Nov 22 '19 at 4:32
  • I did use the semaphore before, with no result. Also, I had a different implementation of destroy() which used a while loop, same result. – bem22 Nov 22 '19 at 4:52
  • 1
    You must take the semaphore. Also update question with while loop code. – Murray Jensen Nov 22 '19 at 4:59
0

enter image description here

I have just solved it. Murray Jensen was right. It was the recursion that was killing my kernel.

Could somebody explain why I spent 7 hours to learn this? What is the maximum recursion depth C is capable of in reality? I read an article this morning ad it was saying 523756 I read it here, scroll down to C.

Here's my deallocator. Zero leaks as you may have noticed.

static void destroy2(list *l) {
    Node *_current = l->node;
    Node *_next;
    while(_current) {
        _next = _current->next;
        kfree(_current->string);
        kfree(_current);
        _current = _next;
    }
}

Another bad thing about the recursive approach I have in the main post is that randomly it would skip kfree-ing 2 to 4 nodes.

For anyone interested in my leak check report: I am using an open-source tool I discovered on github at https://github.com/euspecter/kedr. Comes with no guarantee, but it is very helpful. You don't need to recompile your kernel.

2
  • 1
    The kernel module unloading may (?) be done in kernel context which would mean there is very little space for the stack (measured in kilobytes? unless they have increased it in later kernels) - 10000 calls could easily overrun this ... but I thought they had added a protected page at the end of the stack so these overflows could be caught - I wouldn't have expected this to crash the kernel (but a lot of overhead in this, maybe it is configurable?). PS your while loop "deallocator" is correct :-) – Murray Jensen Nov 23 '19 at 2:45
  • Thank you, mate. You really helped me understand this problem. You asked relevant questions and gave me feedback that drove me to the solution. – bem22 Nov 23 '19 at 18:48

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.