2

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 +++
9
  • Double check what information is sent to device_ioctl(). The error is telling you it doesn't think that is proper for a message. See What does the error stty: 'standard input': Inappropriate ioctl for device Can't obtain kerberos tickets mean? Have you run strace on sender to see if any additional information is available? Commented Jun 5, 2023 at 8:40
  • did you implement the handling for write operation ? The first clue is in the line ioctl(0, _IOC(_IOC_WRITE, 0xeb, 0, 0x4), 0x1) = -1 ENOTTY (Inappropriate ioctl for device) Commented Jun 5, 2023 at 9:25
  • Yes, I edited the question and added my write implementation. I any case, write is being called from the user program only after ioctl is issued. Commented Jun 5, 2023 at 9:30
  • Can you please re-edit your question to include the code - insert_channel(root, curr); what is happening there? minor = iminor(file->f_inode); can you confirm if that device does exist, also, root = minors[minor]; where minors is declared as struct channel *minors[257]; is that an array of pointers to struct channel ? Was this allocated? Should it be just struct channel minors[257]; ? Commented Jun 5, 2023 at 10:20
  • Edited. The declaration of minors minors is at the very beginning of the module, right after #include statements. Commented Jun 5, 2023 at 10:42

1 Answer 1

1

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.

Sign up to request clarification or add additional context in comments.

2 Comments

well spotted! Also shows how the subtlety can be easily missed.
@t0mm13b The entire construct of cramming the assignment into the if statement is soooo bug prone it's indefensible IMO. "It's idiomatic" - yeah, it's an idiom that dates back to the days where it was also idiomatic to put lead in gasoline. Why anyone continues to needlessly teach polluting code with such a bug-prone construct simply because that's the way it was done 40+ years ago is beyond me.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.