On Tue, 19 Sep 2017 15:54:22 +0200, Meng Xu wrote:
Hi Takashi,
Thanks for the reply. In my opinion, many security issues are in fact unhandled corner cases and this could be one.
In the first fetch, get_user(hm->h.size, (u16 __user *)puhm), only 2 bytes from puhm are copied in and later it is ensured that hm->h.size (which is also hm->m0.size given hm is a union) is no larger than sizeof(*hm). However, this relation is broken after the second fetch, copy_from_user(hm, puhm, hm->h.size).
As a concrete example, a user could put 0x000A when the first fetch happens which make hm->h.size <= sizeof(*hm) and later races to change it to 0xFFFF in the second fetch. What makes it even worse is this call: hpi_send_recv_f(&hm->m0, &hr->r0, file), which sends &hm->m0 to a lot of destinations. If any of the downstream functions assumes that hm->m0.size <= sizeof(*hm) which is actually not, an exploit can be constructed.
In fact, similar issues have caused vulnerabilities before as in https://bugzilla.kernel.org/show_bug.cgi?id=116651 https://bugzilla.kernel.org/show_bug.cgi?id=120131, and more recently the fix in sched/perf https://marc.info/?l=linux-kernel&m=150401225812533&w=2
Feel free to let us know your opinion.
OK, now it's clearer, it's about the overwrite by the second copy_from_user() call. I took the patch as is now.
thanks,
Takashi