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 <[email protected]> : > 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. <[email protected]> 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 >> [email protected] >> http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies >> > > -- *"Do or do not. There is no try"* *Yoda Master*
_______________________________________________ Kernelnewbies mailing list [email protected] http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
