Accessing pointers inside struct passed as argument to ioctl calls

[Daniel.]

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 at opteya.com> 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
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20150928/74a19d64/attachment-0001.html