Accessing pointers inside struct passed as argument to ioctl calls

[Daniel.]

Okay, thank you for the tips Kenneth! This is not real code but the case
is, so I will do that checks that you pointed out!! Thanks again!

Best regards,
- dhs

2015-09-28 14:09 GMT-03:00 Kenneth Adam Miller <kennethadammiller at gmail.com>
:

> 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
>>
>
>


-- 
*"Do or do not. There is no try"*
  *Yoda Master*
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20150928/631d1430/attachment.html