netdev - Re: [patch 1/1][CORE] resend - fix free_netdev on register

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 30 Oct 2007 21:52:43 -0700
From:	Stephen Hemminger <shemminger@...ux-foundation.org>
To:	David Miller <davem@...emloft.net>
Cc:	dlezcano@...ibm.com, netdev@...r.kernel.org
Subject: Re: [patch 1/1][CORE] resend - fix free_netdev on register_netdev
 failure

On Tue, 30 Oct 2007 15:38:30 -0700 (PDT)
David Miller <davem@...emloft.net> wrote:

> From: Daniel Lezcano <dlezcano@...ibm.com>
> Date: Tue, 30 Oct 2007 17:19:26 +0100
> 
> > Point 1:
> > The unregistering of a network device schedule a netdev_run_todo.
> > This function calls dev->destructor when it is set and the
> > destructor calls free_netdev.
> > 
> > Point 2:
> > In the case of an initialization of a network device the usual code
> > is:
> >  * alloc_netdev
> >  * register_netdev
> >     -> if this one fails, call free_netdev and exit with error.
> > 
> > Point 3:
> > In the register_netdevice function at the later state, when the device
> > is at the registered state, a call to the netdevice_notifiers is made.
> > If one of the notification falls into an error, a rollback to the
> > registered state is done using unregister_netdevice.
> > 
> > Conclusion:
> > When a network device fails to register during initialization because
> > one network subsystem returned an error during a notification call
> > chain, the network device is freed twice because of fact 1 and fact 2.
> > The second free_netdev will be done with an invalid pointer.
> > 
> > Proposed solution:
> > The following patch move all the code of unregister_netdevice *except* 
> > the call to net_set_todo, to a new function "rollback_registered".
> > 
> > The following functions are changed in this way:
> >  * register_netdevice: calls rollback_registered when a notification fails
> >  * unregister_netdevice: calls rollback_register + net_set_todo, the call
> >                          order to net_set_todo is changed because it is the
> >                          latest now. Since it justs add an element to a list
> >                          that should not break anything.
> > 
> > Signed-off-by: Daniel Lezcano <dlezcano@...ibm.com>
> 
> Looks good, applied.

Agreed, but I wish the unwind code wouldn't have to be separate function.

-- 
Stephen Hemminger <shemminger@...ux-foundation.org>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html