Why does copying a struct from user space fails in an ioctl command?

Question

I'm developing a device driver and need to make use of IOCTL. Unfortunately I cannot copy a struct from user space. Here is the code (simplified, error handling removed):

Structure

struct secvault_createoptions {
    int secvaultId;
    long dataSize;
    char key[SECVAULT_KEYSIZE];
};

Application

void createSecvault(int secvaultId)
{
    struct secvault_createoptions creationOptions;
    /* fill with data */
    sendIoctlCommand(SECVAULT_IOCTL_CREATE, &creationOptions);
}

void sendIoctlCommand(int command, void *arg)
{
    FILE *stream;
    int fd, err;

    stream = fopen(SECVAULT_DEV_CONTROL, "r");
    fd = fileno(stream);
    ioctl(fd, command, arg);
    fclose(stream);
}

Kernel Module

int control_device_ioctl(struct inode *node, struct file *filp, unsigned int cmd, unsigned long arg)
{
    struct secvault_createoptions creationOptions;
    int returnCode;

    switch (cmd)
    {
        case SECVAULT_IOCTL_CREATE:
            if (copy_from_user(&creationOptions, (void*)arg, sizeof(struct secvault_createoptions)) != sizeof(struct secvault_createoptions))
            {
                /* Always this branch gets executed */
                printk(KERN_ALERT "Copying secure vault creation options from user space failed.\n");
                returnCode = -EFAULT;
                break;
            }
            printk(KERN_ALERT "2 IOCTL create request on control device received: secvaultId = %d, dataSize = %ld.\n",
                creationOptions.secvaultId, creationOptions.dataSize);

            returnCode = createDataDevice(&creationOptions);
            break;
    }
    return returnCode;
}

Best Regards,
Oliver Hanappi

score 4 · Accepted Answer · answered Jan 08 '11 at 17:01

4

Your copy_from_user call is wrong. It does not return the amount of bytes copied, but the number of bytes that were not copied. What you want is

if (copy_from_user(...) != 0)
        return -EFAULT;

(You can skip the assignment to ret in your snippet.)

answered Jan 08 '11 at 17:01

user562374

3,817
1
22
19

The usual idiom is if (copy_from_user(...)) return -EFAULT; – mpe Jan 09 '11 at 03:22

score 3 · Answer 2 · answered Jan 08 '11 at 17:01

3

copy_from_user() returns the number of bytes that could not be copied. So you should expect 0 for success, rather than sizeof(struct secvault_createoptions).

answered Jan 08 '11 at 17:01

Matthew Slattery

45,290
8
103
119

score 0 · Answer 3 · edited Sep 02 '14 at 06:50

You should modify the statement as ,

  if (copy_from_user(&creationOptions, (void*)arg, sizeof(struct secvault_createoptions)) != 0)
            {
                /* Always this branch gets executed */
                printk(KERN_ALERT "Copying secure vault creation options from user space failed.\n");
                returnCode = -EFAULT;
                break;
            }

Because copy_from_user always returns 0 after successful completion.

Please refer to this for more details.

Why does copying a struct from user space fails in an ioctl command?

3 Answers3