svn commit: r310180 - head/sys/net

[Mateusz Guzik]

On Fri, Dec 16, 2016 at 03:55:41PM -0700, Alan Somers wrote:
> On Fri, Dec 16, 2016 at 3:45 PM, Mateusz Guzik <mjguzik at gmail.com> wrote:
> > On Fri, Dec 16, 2016 at 10:39:31PM +0000, Alan Somers wrote:
> >> Author: asomers
> >> Date: Fri Dec 16 22:39:30 2016
> >> New Revision: 310180
> >> URL: https://svnweb.freebsd.org/changeset/base/310180
> >>
> >> Log:
> >>   Fix panic during lagg destruction with simultaneous status check
> >>
> >>   If you run "ifconfig lagg0 destroy" and "ifconfig lagg0" at the same time a
> >>   page fault may result. The first process will destroy ifp->if_lagg in
> >>   lagg_clone_destroy (called by if_clone_destroy). Then the second process
> >>   will observe that ifp->if_lagg is NULL at the top of lagg_port_ioctl and
> >>   goto fallback: where it will promptly dereference ifp->if_lagg anyway.
> >>
> >>   The solution is to repeat the NULL check for ifp->if_lagg
> >>
> >
> > I don't understand how this solves the problem. What prevents the object
> > from getting freed after the pointer got NULLified? That is, it seems
> > the patch turns a null pointer deref into a use-after-free.
> >
> > There seems to be a refcounting issue here.
> >
> > That said, I only did cursory reading.
> 
> What object are you talking about?  The problem solved by this commit
> is the null-pointer dereference in the thread calling lagg_port_ioctl,
> for example "ifconfig lagg0".

I misread the patch, sorry for the noise.

-- 
Mateusz Guzik <mjguzik gmail.com>