Re: Accessing pointers inside struct passed as argument to ioctl calls

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Yann, thank you, as I said this isn't real code, I just use to show my case. Anyway I will take the considerations in account. Thank you so much! And this get_user_page is new to me, thanks for pointing me out, I will read about it.

The real thing is a driver to nrf24l01+ driver from Nordic. I may use this non copying aproach to exchange lot of frames without copying. This would improve driver's performance. :)

Best regards!
-dhs

Em 28/09/2015 17:08, "Yann Droneaud" <ydroneaud@xxxxxxxxxx> escreveu:
Hi,

Le lundi 28 septembre 2015 à 13:51 -0300, Daniel. a écrit :
> Hi all, I have a doubt about using pointers inside structs that are
> passed (as pointers) to ioctl argument. Since pointers passed from
> userspace can't be trusted, I need to copy they to kernel before
> accessing they. In this case I have a pointer inside a struct that is
> passed to the ioctl call also as pointer. So I need to copy the whole
> struct to kernel space and only then dereference the pointer. If this
> is true, two copy_from_user is needed right?
>
> Suppose I have a struct like this
>
> struct myioctl_arg {
>     char *tx_buf;

__u8 __user *tx_buf;

>     int tx_siz;

__u32 tx_siz;

> }
>
> and this ioctl definition
>
> #define MYIOCTL_TX _IOWR(MY_MAGIC, MY_BASE, struct myioctl_arg *);
>

#define MYIOCTL_TX _IOWR(MY_MAGIC, MY_BASE, struct myioctl_arg);

> At userspace program I do something like this:
>
> struct myioctl_arg arg = { .tx_buf = somebuf, .tx_siz = 32 }
> ioctl(fd, MYIOCTL_TX, &arg);
>
> And in my ioclt implementation
> static ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> {
>     struct myioctl_arg karg;
>     char   kbuf[100];
>     ...
>     case MYIOCTL_TX:
>           copy_from_user(&karg, arg, sizeof(karg));

        if (copy_from_user(&karg, (const void __user *)arg, sizeof(karg))
                return -EFAULT;

        if (!karg.tx_siz)
                return -EINVAL;

        if (karg.tx_siz > sizeof(kbuf))
                return -EINVAL;

>           copy_from_user(kbuf, karg.tx_buf, karg.tx_siz);

        if (copy_from_user(kbuf, karg.tx_buf, karg.tx_siz))
                return -EFAULT;

>           // now kbuf has the same contents of somebuf in userspace
>    ...
> }
>
>
> I'm in doubt if this is the right way to access the userspace tx_buf.
> Since the .tx_buf from arg is a userspace pointer, accessible from
> userspace pointer arg, I can't type arg->tx_buf directly, doing this
> is dereferencing a userspace pointer inside kernel. So I need to copy
> the whole struct and dereference from the kernel space copy of it. So
> this two calls of copy_from_user() is the right way to do it? They are
> needed at all or is there better way of doing it?
>

They are needed. Add __user annotation, install sparse and build with
make C=1.

For the second one, if the tx_buf is large enough, you may be well using
something like get_user_page() to pin userspace memory inside the kernel
memory so that you don't have to copy the memory to access it.

Regards.

--
Yann Droneaud
OPTEYA


_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@xxxxxxxxxxxxxxxxx
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

[Index of Archives]     [Newbies FAQ]     [Linux Kernel Mentors]     [Linux Kernel Development]     [IETF Annouce]     [Git]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux SCSI]     [Linux ACPI]
  Powered by Linux