5
\$\begingroup\$

My Linux kernel version is

Linux version 5.0.0-29-generic 

I'm currently learning how to write kernel modules for Linux and implemented this simple module which creates a new procfs entry and allows some data to be read from it.

The module consists of the following files:

init.c:

#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/proc_fs.h>
#include "pfs_regfile.h"

MODULE_LICENSE("GPL");

static struct proc_dir_entry *pde_regfile;

static struct file_operations fops_regfile = {
    .owner = THIS_MODULE,
    .open = pfs_open,
    .read = pfs_device_read,
    .release = pfs_release
};

static int __init init_md(void){
    pde_regfile = proc_create(PROC_REGFILE_NAME, 0, NULL, &fops_regfile);
    if(!pde_regfile){
        remove_proc_entry(PROC_REGFILE_NAME, NULL);
        printk(KERN_ALERT "Error: could not initialize procfs entry /proc/%s", PROC_REGFILE_NAME);
        return -ENOMEM;
    }
    return 0;
}

static void __exit exit_md(void){
    remove_proc_entry(PROC_REGFILE_NAME, NULL);
}

module_init(init_md);
module_exit(exit_md);

pfs_regfile.h:

#ifndef PFS_REGFILE_H
#define PFS_REGFILE_H

#include <linux/fs.h>

#define PROC_REGFILE_NAME "pfs_regfile"

int pfs_open(struct inode*, struct file *);

int pfs_release(struct inode *, struct file *);

ssize_t pfs_device_read(struct file *, char __user * buffer, size_t, loff_t *);

#endif //PFS_REGFILE_H

pfs_regfile.c:

#include <linux/module.h>
#include <linux/uaccess.h>
#include "pfs_regfile.h"

static char data[5000];
static char *current_ptr;
int pfs_open(struct inode* in, struct file *filp){
    for (int i = 0; i < sizeof(data) - 1; i++){
        data[i] = 'a';
    }
    current_ptr = data;
    try_module_get(THIS_MODULE);
    return 0;
}

int pfs_release(struct inode *in, struct file *filp){
    module_put(THIS_MODULE);
    return 0;
}

ssize_t pfs_device_read(struct file *file, char __user * buffer, size_t len, loff_t *off){
    size_t pos = (size_t)(current_ptr - data);
    size_t to_copy = pos + len < sizeof data ? len : sizeof data - pos;
    unsigned long not_copied = copy_to_user(buffer, current_ptr, to_copy);
    if(not_copied){
        printk(KERN_ALERT "Unknown error occurred when copying data into userspace\n");
    }
    size_t copied = to_copy - (size_t) not_copied;    
    current_ptr += copied;
    return (ssize_t) copied;
}

I'm not sure if this module should work in all cases and its implementation is sort of "idiomatic". I'm especially concerned about concurrency. I tried to access it concurrently from different processes (cat /proc/pfs_regfile), and it worked fine.

\$\endgroup\$
  • \$\begingroup\$ i < sizeof(data) - 1 - why leave the last byte uninitialized? \$\endgroup\$ – Reinderien Oct 14 '19 at 17:36
2
\$\begingroup\$

memset

for (int i = 0; i < sizeof(data) - 1; i++){
    data[i] = 'a';
}

shouldn't be needed. Just use memset from string.h, which is indeed available in the kernel.

sizeof

You have this:

int i = 0; i < sizeof(data) - 1; i++

but also this:

pos + len < sizeof data ? len : sizeof data - pos

Today I learned that the parens are needed on types and optional on expressions:

The operator has a single operand, which is either an expression or a data type cast. A cast is a data type enclosed in parenthesis.

The only syntax I've ever seen is with parens. You should at least stay internally consistent.

Buffer sizes

5000 is a little unusual, especially inside the kernel. Unless there are hidden motivations, consider just making it 1024*4 == 4096.

\$\endgroup\$
  • \$\begingroup\$ I was not sure if it legitimate to use standard library function from Kernel Space. That's why I used the loop. \$\endgroup\$ – St.Antario Oct 15 '19 at 6:27

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.