<div dir="ltr">You are right, and thank you for bringing this to the mailing list to be sure about it.<div><br></div><div>There are several catastrophic vulnerabilities I can see waiting to happen.</div><div><br></div><div>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.</div><div><br></div><div>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.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 28, 2015 at 12:51 PM, Daniel. <span dir="ltr"><<a href="mailto:danielhilst@gmail.com" target="_blank">danielhilst@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi all, I have a doubt about using pointers inside structs that are<br>
passed (as pointers) to ioctl argument. Since pointers passed from<br>
userspace can't be trusted, I need to copy they to kernel before<br>
accessing they. In this case I have a pointer inside a struct that is<br>
passed to the ioctl call also as pointer. So I need to copy the whole<br>
struct to kernel space and only then dereference the pointer. If this<br>
is true, two copy_from_user is needed right?<br>
<br>
Suppose I have a struct like this<br>
<br>
struct myioctl_arg {<br>
char *tx_buf;<br>
int tx_siz;<br>
}<br>
<br>
and this ioctl definition<br>
<br>
#define MYIOCTL_TX _IOWR(MY_MAGIC, MY_BASE, struct myioctl_arg *);<br>
<br>
At userspace program I do something like this:<br>
<br>
struct myioctl_arg arg = { .tx_buf = somebuf, .tx_siz = 32 }<br>
ioctl(fd, MYIOCTL_TX, &arg);<br>
<br>
And in my ioclt implementation<br>
static ioctl(struct file *fp, unsigned int cmd, unsigned long arg)<br>
{<br>
struct myioctl_arg karg;<br>
char kbuf[100];<br>
...<br>
case MYIOCTL_TX:<br>
copy_from_user(&karg, arg, sizeof(karg));<br>
copy_from_user(kbuf, karg.tx_buf, karg.tx_siz);<br>
// now kbuf has the same contents of somebuf in userspace<br>
...<br>
}<br>
<br>
<br>
I'm in doubt if this is the right way to access the userspace tx_buf.<br>
Since the .tx_buf from arg is a userspace pointer, accessible from<br>
userspace pointer arg, I can't type arg->tx_buf directly, doing this<br>
is dereferencing a userspace pointer inside kernel. So I need to copy<br>
the whole struct and dereference from the kernel space copy of it. So<br>
this two calls of copy_from_user() is the right way to do it? They are<br>
needed at all or is there better way of doing it?<br>
<br>
Cheers,<br>
<span class="HOEnZb"><font color="#888888">--<br>
"Do or do not. There is no try"<br>
Yoda Master<br>
<br>
_______________________________________________<br>
Kernelnewbies mailing list<br>
<a href="mailto:Kernelnewbies@kernelnewbies.org">Kernelnewbies@kernelnewbies.org</a><br>
<a href="http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies" rel="noreferrer" target="_blank">http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies</a><br>
</font></span></blockquote></div><br></div>