Messages in this thread | | | From | (Eric W. Biederman) | Date | Thu, 22 Mar 2012 16:02:46 -0700 | Subject | Re: [3.3-rc7] sys_poll use after free (hibernate) |
| |
Lucas De Marchi <lucas.demarchi@profusion.mobi> writes:
> On Thu, Mar 22, 2012 at 6:31 PM, Eric W. Biederman
>> It looks like it was a combination of the fuzzer doing silly things >> and a removed ctl_table entry being poisoned and having .poll set >> to 6b6b6b6b6b6b6b6b so the guard against calling poll when it is >> nonsense did not trigger. So your patch should be sufficient >> for now. > > What I understood afterwards was: > > 1. fuzzer calling poll() on files that did support poll > 2. modules that created that sysctl entries were removed > 3. 'table' was entirely removed (not ->poll).
I just grepped the kernel for ctl_table_poll and DEFINE_CTL_TABLE_POLL. There are only the two original users of hostname and domainname.
The problem very much had to be that ctl_table was freed and poisoned but we still pointed to it, and we were not using the grab_header idiom to ensure we did not use an expired ctl_table entry.
Which means that it was any ctl_table being add/removed. Probably in this case the per cpu scheduler sysctl table entries that get added/removed whenever we logically add/remove a cpu.
I expect what happened is that the fuzzer opened the sysctl file some time before it was removed and then sometime after the entry was removed (but before the memory was reused) called select/poll on that file descriptor. Since the ctl_table was poisoned ->poll was 6b6b6b6b6b6b6b6b and so we passed the checks for a non NULL ->poll and we proceed to do nonsense things that caused the kernel oops in proc_sys_poll.
>> Long term we still need a version of poll that is safe to use >> with modules. > > I think the way it's now (with my patch taken by Andrew) is safe for > having poll() with modules.
No it is not.
The problem is that proc_sys_poll is non-blocking. It is called primarily to place the system on a wait queue. But notice that if you place the caller on a wait_queue in proc_sys_poll and return then we may call unregister_sysctl_table while and remove the sysctl while someone still is on the wait queue. Sleeping on a wait_queue that has been freed is so bizarre I don't want to think about the failure modes.
sysfs solves this problem by tracking openers and has it's wait_queue in the per opener structure. That same logic needs to be mirrored in sysctl for poll to be safe on any sysctl table entry that can be removed.
I believe a correct fix would remove the .poll field in struct ctl_table, remove struct ctl_table_poll entirely and modify the signature of proc_sys_poll_notify to be: void proc_sys_poll_notify(struct ctl_table_header *head, struct ctl_table *table);
Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |