netdev - Re: [PATCH] various dst_ifdown routines to catch refcounting bugs

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]

Message-Id: <20070927.124438.56162804.davem@davemloft.net>
Date:	Thu, 27 Sep 2007 12:44:38 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	ebiederm@...ssion.com
Cc:	den@...nvz.org, devel@...nvz.org, netdev@...r.kernel.org,
	containers@...ts.osdl.org
Subject: Re: [PATCH] various dst_ifdown routines to catch refcounting bugs

From: ebiederm@...ssion.com (Eric W. Biederman)
Date: Thu, 27 Sep 2007 10:27:43 -0600

> "Denis V. Lunev" <den@...nvz.org> writes:
> 
> > Moving dst entries into init_net.loopback_dev is not a good thing.
> > This hides obvious and non-obvious ref-counting bugs.
> 
> Acked-by: "Eric W. Biederman" <ebiederm@...ssion.com>

Patch applied.

> I do have a question I would like to bring up, because I like avoiding
> explicit references to loopback_dev when I can.
> 
> /* Dirty hack. We did it in 2.2 (in __dst_free),
>  * we have _very_ good reasons not to repeat
>  * this mistake in 2.3, but we have no choice
>  * now. _It_ _is_ _explicit_ _deliberate_
>  * _race_ _condition_.
>  *
>  * Commented and originally written by Alexey.
>  */
> 
> What is the race that is talked about in that comment.  Can we just
> assign NULL instead of the loopback device when we bring a route down.
> My gut feeling is that something like:
> 	dst->input = dst->output = dst_discard;
> may be enough.    But I don't know where the deliberate race is.

The packet output path accesses the cached route device
asynchronously, and we are resetting the device to be loopback without
any synchronization whatsoever.  None is in fact possible, and we
don't want to add it because that would be way too expensive.

So another thread on the system can either see the original device or
the loopback one.

It all works out because as the device goes down we'll purge any
packets queued into the transmit queue and packet scheduler for that
device.
-
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