Accessing pointers inside struct passed as argument to ioctl calls

[Kenneth Adam Miller]

You are right, and thank you for bringing this to the mailing list to be
sure about it.

There are several catastrophic vulnerabilities I can see waiting to happen.

First, you should be sure that the pointer that they passed in is checked,
as in the pointer to the buffer should only reside in the mapped memory for
their process.

Second, you trust the size that they pass in with tx_size. You should take
the minimum of tx_siz and size of your kbuf (which is a static 100). Be
careful that when you record a static value to stub out this size, a number
is implicitly by default a signed word size int to the compiler. So a
#define would be signed, but if  you change the type of int tx_siz, be
careful. Also, likely you don't want negative buffer sizes. size_t or
uint16_t should suit your purposes.

On Mon, Sep 28, 2015 at 12:51 PM, Daniel. <danielhilst at gmail.com> wrote:

> 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;
>     int tx_siz;
> }
>
> and this ioctl definition
>
> #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));
>           copy_from_user(kbuf, karg.tx_buf, karg.tx_siz);
>           // 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?
>
> Cheers,
> --
> "Do or do not. There is no try"
>   Yoda Master
>
> _______________________________________________
> Kernelnewbies mailing list
> Kernelnewbies at kernelnewbies.org
> http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20150928/20330fe4/attachment.html