|Summary:||Wireless ioctls fail if struct iwreq is at the end of a memory mapping|
|Product:||Networking||Reporter:||Robert O'Callahan (robert)|
|Component:||Wireless||Assignee:||Johannes Berg (johannes)|
complete reduced testcase
Description Robert O'Callahan 2017-05-24 02:21:26 UTC
Created attachment 256697 [details] complete reduced testcase If you do, say struct iwreq* req = ...; int ret = ioctl(sockfd, SIOCGIWNAME, req); the ioctl will fail with EFAULT if req points to valid memory that is exactly sizeof(struct iwreq). Full testcase attached. The problem is that dev_ioctl() does a copy_from_user(sizeof(struct ifreq)). On x86-64 that's 40 bytes, but sizeof(struct iwreq) is only 32 bytes. This isn't a huge deal, but it does mean that user code that stores struct iwreq on the heap can randomly fail once in a blue moon, and that's bad. Alternatively it could be documented somewhere that user code should actually pass in a struct ifreq and cast it to struct iwreq for all usage, but that's horrible.
Comment 1 Johannes Berg 2017-06-14 06:18:45 UTC
Thanks for the report, and sorry it went unnoticed for this long. Do you want to propose a patch, since you've already analysed the problem so deeply?
Comment 2 Johannes Berg 2017-06-14 07:41:35 UTC
It's somewhat involved actually, I'm fixing it.
Comment 3 Robert O'Callahan 2017-06-14 10:37:38 UTC
Comment 4 Johannes Berg 2017-06-14 11:31:04 UTC
I have a fix, want to test it? Will attach a minimal version of the fix.
Comment 5 Robert O'Callahan 2017-06-14 11:32:24 UTC
I'll test it but I doubt my testing will add anything over yours ...
Comment 7 Johannes Berg 2017-06-14 11:39:19 UTC
Just to give you an idea of what kind of "gem" you found here - this bug was introduced in mainline in 2.1.15, almost exactly 20.5 years (!!) ago today (released Dec 12, 1996).
Comment 8 Johannes Berg 2017-06-14 11:42:55 UTC
I'm still waiting for the kernel to finish compiling before I can test it, but I'm pretty confident :)
Comment 9 Johannes Berg 2017-06-14 11:50:49 UTC
It finished, and I can confirm the bug is fixed: without patch: # /tmp/test wlan0 Failed SIOCGIWNAME: Bad address with patch: # /tmp/test wlan0 Failed SIOCGIWNAME: No such device yeah, I didn't even have one, but it doesn't matter :-) with a device existing I get # /tmp/test wlan0 wireless protocol name: IEEE 802.11
Comment 10 Robert O'Callahan 2017-06-14 11:51:19 UTC
Comment 11 Johannes Berg 2017-06-14 11:56:15 UTC
I'll also say that you should probably stop using wireless ioctls and use nl80211 instead :-)