Creating procfs entry on linux in Kernel Module

Question

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.

Answer 1

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.