lkml.org 
[lkml]   [2012]   [Mar]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [3.3-rc7] sys_poll use after free (hibernate)
    On Thu, Mar 22, 2012 at 6:31 PM, Eric W. Biederman
    <ebiederm@xmission.com> wrote:
    > Lucas De Marchi <lucas.demarchi@profusion.mobi> writes:
    >
    >> On Sun, Mar 18, 2012 at 4:27 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
    >>> On Sun, Mar 18, 2012 at 12:02:04PM -0700, Linus Torvalds wrote:
    >>>> and that load is from
    >>>>
    >>>>     poll_wait(filp, &table->poll->wait, wait);
    >>>>
    >>>> where the testing of %rsi and %rcx are the "if (p && wait_address)"
    >>>> check in poll_wait(), and %rsi is "table->poll" if I read it all
    >>>> correctly.
    >>>>
    >>>> And the 6b6b6b6b6b6b6b6b pattern is obviously POISON_FREE, so
    >>>> apparently 'table' has already been freed.
    >>>>
    >>>> I suspect the whole sysctl 'poll' code is seriously broken, since it
    >>>> seems to depend on those ctl_table pointers being stable over the
    >>>> whole open/close sequence, but if somebody unregisters the sysctl,
    >>>> it's all gone. The ctl_table doesn't have any refcounting etc, and I
    >>>> suspect that your hibernate sequence ends up unregistering some sysctl
    >>>> (perhaps as part of a module unload?)
    >>
    >> How could that happen if the only files that support poll  right now
    >> on sysctl are kernel/hostname and kernel/domainname?
    >>
    >>>
    >>> Ewww...  The way it was supposed to work (prio to ->poll() madness) was
    >>> that actual IO gets wrapped into grab_header()/sysctl_head_finish()
    >>> pair.  proc_sys_poll() doesn't do it, so yes, that post-mortem is
    >>> very likely to be correct.
    >>
    >> Yes, it  seems like I forgot to call grab_header() there, sorry for
    >> that. I'll prepare a patch and send you later today. I just wonder
    >> what is happening to reach that code... :-/
    >
    > 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).

    >
    > 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.


    Lucas De Marchi
    --
    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/

    \
     
     \ /
      Last update: 2012-03-22 23:15    [W:0.044 / U:30.948 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site