Re: [3.3-rc7] sys_poll use after free (hibernate)

[ebiederm@xmission.com (Eric W. Biederman)]

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