Bug 195869
| 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) |
| Status: | CLOSED CODE_FIX | ||
| Severity: | normal | ||
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 4.12rc12 | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
| Attachments: |
complete reduced testcase
tentative fix |
||
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? It's somewhat involved actually, I'm fixing it. Thanks! I have a fix, want to test it? Will attach a minimal version of the fix. I'll test it but I doubt my testing will add anything over yours ... Created attachment 256997 [details]
tentative fix
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). I'm still waiting for the kernel to finish compiling before I can test it, but I'm pretty confident :) 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 Great! I'll also say that you should probably stop using wireless ioctls and use nl80211 instead :-) OK. I found this while working on rr (http://rr-project.org/), so this is about recording the execution of applications using those ioctls, not really using them ourselves :-). Thanks again. |
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.