Search code examples
cvirtual-machinekernel-moduleioctl

Why am I getting "Inappropriate ioctl for device" error in my kernel module?


Inappropriate ioctl for device error

Hey, I'm having trouble trying to build a kernel module. It's a character device module named message_slot. According to the instructions, it should support up to 256 minors (all have the same major- 235). Each minor represents a different device file and can have up to 2^20 channels. In order to do so I used a list of binary trees, such that each entry represents a different device file.

After successfully issuing: make (with a generic Makefile, same as the Makefile my peers and my TA used)

sudo insmod message_slot.ko (I have a printk statement in my module init function which I see in the kerenel log after this command, so I know registration succeeded)

sudo mknod /dev/slot1 c 235 1

sudo chmod o+rw /dev/slot1

and after compiling a user program named message_sender.c with -o sender I'm now ready to write a message to the device file (minor num 1) with:

./sender /dev/slot1 1 message

(message_sender.c main function is getting 3 arguments - the device file, minor and the message to write)

After issuing, I'm getting the following message: Ioctl failed: Inappropriate ioctl for device

And I really don't understand why.

Attached are parts of the user program message_sender.c, the module message_slot.c and the header message_slot.h:

message_sender.c

    #include <fcntl.h>
    #include <unistd.h>
    #include <sys/ioctl.h>
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>

    #include "message_slot.h"


    int main(int argc, char** argv){
    int fd;
    unsigned int channel_num;
    char *msg;
    if (argc != 4){
        perror("Error- Invalid number of arguments");
        exit(1);
    }
    if ((fd = open(argv[1], O_RDWR) < 0)){
        perror("Couldnt open the specified file");
        exit(1);
    }
    channel_num = atoi(argv[2]);
    printf("channel num: %d", channel_num);
    if (ioctl(fd,MSG_SLOT_CHANNEL, channel_num) < 0){
        perror("Ioctl failed");
        exit(1);
    }
    msg = argv[3];
    if (write(fd, msg, strlen(msg)) != strlen(msg)){
        perror("Error occured while writing the message");
        exit(1);
    }
    close(fd);
    exit(0);
}

message_slot.c

#undef __KERNEL__
#define __KERNEL__
#undef MODULE
#define MODULE


#include <linux/kernel.h>  
#include <linux/module.h>   
#include <linux/fs.h>
#include <linux/init.h>      
#include <linux/uaccess.h>  
#include <linux/string.h> 
#include <linux/slab.h> 
#include "message_slot.h"
MODULE_LICENSE("GPL");

struct channel* search_channel(struct channel*, int);
void insert_channel(struct channel*, struct channel*);
void cleanup_trees(struct channel*);

static struct channel *minors[257];

struct channel* search_channel(struct channel* root, int x) {
    struct channel* curr = root;
    while (curr != NULL) {
        if (curr->channel_number == x) {
            return curr;  // Found the channel with the given channel_number
        } else if (x < curr->channel_number) {
            curr = curr->left;  // Go to the left subtree
        } else {
            curr = curr->right;  // Go to the right subtree
        }
    }
    return NULL;  // Channel with the given channel_number (x) not found
}

void insert_channel(struct channel* root, struct channel* node) {
    struct channel* curr;
    if (root == NULL) {
        // The tree is empty, make the node as the root
        root = node;
    } 
    else {
        curr = root;
        while (1) {
            if ((node->channel_number) < (curr->channel_number)) {
                if (curr->left == NULL) {
                    curr->left = node;  // Insert node as the left child
                    break;
                } 
                else {
                    curr = curr->left;  // Move to the left subtree
                }
            } 
            else {
                if (curr->right == NULL) {
                    curr->right = node;  // Insert node as the right child
                    break;
                } 
                else {
                    curr = curr->right;  // Move to the right subtree
                }
            }
        }
    }
}

// A clean up function, which will be called for each entry in minors when we exit the module

void cleanup_trees(struct channel* root) {
    if (root == NULL) {
        return;  // Base case: empty tree or leaf node
    }

    // Post-order traversal to delete nodes recursively
    cleanup_trees(root->left);
    cleanup_trees(root->right);

    // Delete the current node
    kfree(root);
}

//DEVICE FUNCTIONS:
static int device_open(struct inode* inode, struct file* file) {
    file->private_data = NULL; 
    return SUCCESS;
}


static long device_ioctl(struct file* file, unsigned int ioctl_command_id, unsigned long ioctl_param){
    struct channel *curr;
    struct channel *root;
    int minor;
    printk("in ioctl\n");
    if (ioctl_command_id != MSG_SLOT_CHANNEL || ioctl_param == 0){
        printk("Channel/ Command error\n");
        return -EINVAL;
    }
    minor = iminor(file->f_inode);
    root = minors[minor];
    if((curr = search_channel(root, ioctl_param)) == NULL){
        curr = kmalloc(sizeof(struct channel), GFP_KERNEL);
        curr->channel_number = ioctl_param;
        curr->left = NULL;
        curr->right = NULL;
        insert_channel(root, curr);
    }
    file->private_data = (void *)curr;
    return SUCCESS;
}

static ssize_t device_write(struct file *file, const char __user *buffer, size_t length, loff_t *offset){
    struct channel *curr_channel = (struct channel *)file->private_data;
    char *msg;
    int i;
    if (curr_channel == NULL){
        printk("No channel has been set for this file");
        return -EINVAL;
    }
    if (length == 0 || length > MAX_BUF_LEN){
        printk("Invalid message length");
        return -EMSGSIZE;
    }
    if (buffer == NULL){
        printk("Null buffer");
        return -EINVAL;
    }
    msg = curr_channel->message;
    for (i=0; i< length; i++){
        get_user(msg[i],&buffer[i]);
    }
    curr_channel->bytes = length;
    return length;
}

// REST OF DEVICE FUNCTIONS:
.
.
struct file_operations Fops =
    {
        .owner = THIS_MODULE,
        .read = device_read,
        .write = device_write,
        .open = device_open,
        .unlocked_ioctl = device_ioctl,
        .release = device_release,
};

static int __init ms_init(void){
    int rc;
    int i;
    if ((rc = register_chrdev(MAJOR_NUM,DEVICE_NAME,&Fops)) < 0){
        printk("Registration failed");
        return FAIL;
    }
    for (i=0 ; i < 257 ; i ++){
        minors[i] = NULL;
    }
    printk("Registration Successed\n");
    return SUCCESS;
}

//DEVICE RELEASE FUNCTION:
.
.
module_init(slot_init);
module_exit(slot_cleanup);

message_slot.h:

#ifndef MESSAGE_SLOT_H
#define MESSAGE_SLOT_H

#include <linux/ioctl.h>
#define MAJOR_NUM 235
#define MSG_SLOT_CHANNEL _IOW(MAJOR_NUM, 0, unsigned int)
#define DEVICE_NAME "message_slot"
#define MAX_BUF_LEN 128
#define SUCCESS 0
#define FAIL -1

// We will define the struct channel 
// Considering the fact that we keep all channels in a binary tree
// Also required: channel number, the current message

struct channel {
    int channel_number;
    char message[128];
    int bytes;
    struct channel *right;
    struct channel *left;
};
#endif

the printk("in ioctl\n"); statement in the device_ioctl function is not printed to the kernel log, avidencing that after the user is issuing ioctl, we are not getting into the module's ioctl_device implementation.

Any help will be appreciated!

EDIT

here is the output of strace on sender:

student@OS23:~/ex3_XXXXXXX06$ strace ./sender /dev/slot1 1 message
execve("./sender", ["./sender", "/dev/slot1", "1", "message"], 0x7ffd541beed8 /* 22 vars */) = 0
brk(NULL)                               = 0x562632988000
access("/etc/ld.so.preload", R_OK)      = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=75005, ...}) = 0
mmap(NULL, 75005, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7fe49a3a1000
close(3)                                = 0
openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\3\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0@>\2\0\0\0\0\0"..., 832) = 832
fstat(3, {st_mode=S_IFREG|0755, st_size=1905632, ...}) = 0
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fe49a39f000
mmap(NULL, 1918592, PROT_READ, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7fe49a1ca000
mmap(0x7fe49a1ec000, 1417216, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x22000) = 0x7fe49a1ec000
mmap(0x7fe49a346000, 323584, PROT_READ, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x17c000) = 0x7fe49a346000
mmap(0x7fe49a395000, 24576, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1ca000) = 0x7fe49a395000
mmap(0x7fe49a39b000, 13952, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7fe49a39b000
close(3)                                = 0
arch_prctl(ARCH_SET_FS, 0x7fe49a3a0540) = 0
mprotect(0x7fe49a395000, 16384, PROT_READ) = 0
mprotect(0x562630bb1000, 4096, PROT_READ) = 0
mprotect(0x7fe49a3de000, 4096, PROT_READ) = 0
munmap(0x7fe49a3a1000, 75005)           = 0
openat(AT_FDCWD, "/dev/slot1", O_RDWR)  = 3
fstat(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(0x88, 0), ...}) = 0
brk(NULL)                               = 0x562632988000
brk(0x5626329a9000)                     = 0x5626329a9000
ioctl(0, _IOC(_IOC_WRITE, 0xeb, 0, 0x4), 0x1) = -1 ENOTTY (Inappropriate ioctl for device)
dup(2)                                  = 4
fcntl(4, F_GETFL)                       = 0x402 (flags O_RDWR|O_APPEND)
fstat(4, {st_mode=S_IFCHR|0620, st_rdev=makedev(0x88, 0), ...}) = 0
write(4, "Ioctl failed: Inappropriate ioct"..., 45Ioctl failed: Inappropriate ioctl for device
) = 45
close(4)                                = 0
write(1, "channel num: 1", 14channel num: 1)          = 14
exit_group(1)                           = ?
+++ exited with 1 +++

Solution

  • From the strace output:

    ioctl(0, _IOC(_IOC_WRITE, 0xeb, 0, 0x4), 0x1) = -1
          ^
          What's this?!?!?!
    

    And then there is this code:

    if ((fd = open(argv[1], O_RDWR) < 0)){
    

    That code parses as

    if ((fd = ( open(argv[1], O_RDWR) < 0 ) )){
    

    As the open() call returns 3 per the strace output

    openat(AT_FDCWD, "/dev/slot1", O_RDWR)  = 3
    

    this evaluates to

    fd = ( 3 < 0 )
    

    or

    fd = 0
    

    You are trying to run your ioctl() call on the standard input file descriptor.

    This construct isn't prone to such bugs:

    int fd = open(argv[1], O_RDWR);
    if ( fd < 0 ) {
        .
        .
        .
    

    The lesson here? Don't cram assignments into if statements - it's bug-prone in ways you can't see the bug.

    And with 30+ views as I post this answer, no one else spotted it either.