netdev - Re: Race condition in ipv6 code

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] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 13 Jan 2012 07:02:29 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	Francesco Ruggeri <fruggeri@...stanetworks.com>,
	netdev@...r.kernel.org
Subject: Re: Race condition in ipv6 code

Le jeudi 12 janvier 2012 à 16:11 -0800, Eric W. Biederman a écrit :

> Because the rtnl_lock is broad and we have ABBA deadlocks if we don't in
> particular we hold the rtnl_lock over sysctl registration and removal.
> sysctl removal blocks until all of the callers into the sysctl methods
> namely addrconf_sysctl_forward in this case finish executing.
> 
> CPU 0                                         CPU 1
> 
> rtnl_lock()                                   use_count++
> unregister_netdevice()                           addrconf_ctl_foward
>   unregister_sysctl_table()                        rtnl_lock()
>      wait for use_count of addrconf_ctl_forward
>        to == 0
>      
> 
> I smacked lockdep around so it would warn about the sysfs ones.
> The proc and sysctl ones I never did manage to get lockdep warnings
> but a ABBA deadlock is most definitely possible.
> 
> Any solutions better than simply restarting the system call are welcome.
> 
> Perhaps for these heavy weigh methods we should create a work struct
> and go schedule work to perform the change instead of trying to do the
> work synchronously in the sysctl handler.
> 

This idea was discussed in netconf 2011 (I focused on the ability to
dismantle netdevices at high rates), and I admit I had not implemented
yet.

The problem with rtnl_trylock() is that we make very litle progress if
many tasks are competing for rtnl, and we have no fairness.

The task holding RTNL might be descheduled and all cpus running other
threads spinning in rtnl_trylock()/restart_syscall(). Almost a deadlock.

Maybe waiting RTNL for at least a couple of ms would help, instead of
instantly failing rtnl_trylock() and looping.


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