user7498777
user7498777

Reputation: 3

kernel module killed on using insmod

I am trying to implement cat command as a kernel module. I know file i/o should not be done in a kernel module,. Every time I use insmod module.ko I get the output as killed . How can I fix it? Also how can I improve the code?
kernel version - 4.4
Code:

#include <linux/module.h>   
#include <linux/kernel.h>
#include <linux/unistd.h>
#include <linux/syscalls.h> 
#include <linux/fcntl.h> 
#include <asm/uaccess.h>
#include <linux/init.h> 
#include <linux/fs.h>
#include <linux/file.h> 

static char* argv[10];
static int argc = 1;
module_param_array(argv, int, &argc , 0);

static void cat(int f, char *s)
{
    char buf[8192];
    struct file *file;
    loff_t pos = 0;
    long n;
    mm_segment_t old_fs = get_fs();
    set_fs(KERNEL_DS);

    while((n = vfs_read(f, buf, (long)sizeof buf, &pos)) > 0) 
    {
        //write(1, buf, n);
        file = fget(f);
        if (file) {
            vfs_write(file, buf, (long)sizeof buf, &pos);
            fput(file);
        }


    }
    set_fs(old_fs);
}

static void __init hello_init(void)
{
    int f, i;
    if(argc == 1)
        cat(0, "<stdin>");
    else for(i=1; i<argc; i++)
    {
        f = filp_open(argv[i], O_RDONLY, 0);
        if(f < 0)
            printk("error");
        else{
            cat(f, argv[i]);
            filp_close(f);
        }
    }
}

static void __exit hello_cleanup(void)
{
    printk(KERN_INFO "Cleaning up module.\n");
}

module_init(hello_init);
module_exit(hello_cleanup);

Here is a simplified version of the above code which just reads a file. On commenting out the vfs_read part, I'm able to insmod it, but with vfs_read it shows killed.

#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/syscalls.h>
#include <linux/fcntl.h>
#include <asm/uaccess.h>

static void read_file(char *filename)
{
  int fd;
  char buf[1];
  loff_t f_pos = 0;

  mm_segment_t old_fs = get_fs();
  set_fs(KERNEL_DS);

  fd = filp_open(filename, O_RDONLY, 0);
  if (fd >= 0) {
    printk(KERN_DEBUG);
    while (vfs_read(fd, buf, 1, &f_pos) == 1) //if this
      printk("%c", buf[0]);  //and this is removed I'm able to insmod it
    printk("\n");
    sys_close(fd);
  }
  set_fs(old_fs);
}

static int __init init(void)
{
  read_file("/etc/shadow");
  return 0;
}

static void __exit exit(void)
{ }

module_init(init);
module_exit(exit);

Upvotes: 0

Views: 5934

Answers (1)

Craig Estey
Craig Estey

Reputation: 33601

A few things ...

You're trying to do all this from the module init callback.

argc/argv have nothing in them, so that's a problem (i.e. you could get a segfault).

You could hardwire argc/argv with valid values, but you'd have to recompile and reload the driver with the different files you want to cat.

But it might be better to accept a module parameter and split it up with a strtok equivalent to populate your argv. This is the closest thing to an argv equivalent for a driver [which doesn't really exist in exactly the same way as it does for main in an application].

You wouldn't have to recompile the module, but, you would still have to reload it each time (giving insmod a different argument each time)

However, the real/correct way to do this would be, in the module init, to register your driver as a character device using the standard mechanism. And, likewise, unregister it in the module cleanup routine.

Then, create a /dev entry (e.g. /dev/mycat).

From an application, open /dev/mycat, and write a list of files you want cat'ed to the file descriptor, one per line.

Have the driver's write callback routine, parse down the data in the buffer you're passed (i.e. as if you were implementing your own fgets in userspace), and process the list it gets, performing the cat operation on each file.

Rather than creating a /dev entry, it might be easier to create a /proc entry (e.g. /proc/mycat)--YMMV

The actual mechanism to communicate the file list to the driver is arbitrary. You could use an AF_UNIX socket, a named pipe, hook up to a SysV message queue, etc. but the /dev or /proc solution is probably easier.


UPDATE:

There seems to be another problem in vfs_read. dmesg gives RIP [] vfs_read+0x5/0x130 and fbcon_switch: detected unhandled fb_set_par error

Amongst other things, you are passing an int as the first arg to vfs_read. The first arg needs to be struct file * [just like vfs_write].

Note: If you're building the driver using the standard mechanism, this would have compiled with -Wall and this would have been flagged at compile time.

Also, you're trying to write [the cat output] to the same file you're reading from, even though you opened it with O_RDONLY (i.e. equivalent to userspace cat foobar > foobar). That's because, for the vfs_write, you did file = fget(f). What you really wanted to do was file = fget(1)

For both vfs_read and vfs_write, you need fget/fput pairings. And, they need to be separate:

file_in = fget(f);
file_out = fget(1);

// read/write loop ...
while (1) {
    ...
}

fput(file_in);
fput(file_out);

You do not want to put buf on the stack. This produces a race condition. The [kernel] thread could migrate to another processor while I/O is still pending on the vfs_read [AFAIK]. Other drivers that do similar stuff use kmalloc to get the buffer [and kfree to free it].

Upvotes: 1

Related Questions