netdev - Re: bug in bonding driver

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]

Message-ID: <21160.1269458000@death.nxdomain.ibm.com>
Date:	Wed, 24 Mar 2010 12:13:20 -0700
From:	Jay Vosburgh <fubar@...ibm.com>
To:	"Chris Friesen" <cfriesen@...tel.com>
cc:	netdev@...r.kernel.org, bonding-devel@...ts.sourceforge.net
Subject: Re: bug in bonding driver

Chris Friesen <cfriesen@...tel.com> wrote:
>One of our guys pointed out what appears to be a bug in
>bond_ab_arp_inspect().  There's a chunk of code that looks like this:
>
>	/*
>	 * Give slaves 2*delta after being enslaved or made
>	 * active.  This avoids bouncing, as the last receive
>	 * times need a full ARP monitor cycle to be updated.
>	 */
>	if (!time_after_eq(jiffies, slave->jiffies +
>			   2 * delta_in_ticks))
>		continue;
>
>The catch here is that slave->jiffies may not ever get updated after
>being set initially, and on long-running systems jiffies will overflow.
> That could cause this check to be true for a substantial amount of time
>rather than for just a short period.

	The definition for time_after in include/linux/jiffies.h claims
to handle timer wrapping, but even so, there presumably has to be a
cutoff at which "after" becomes "before" again.  

	Some quick fooling around suggests that if, for example,
slave->jiffies is near the top of the range (ULONG_MAX - a few hundred),
when jiffies gets up to around ULONG_MAX / 2 time_after_eq will flip
from "after" to "before."

	I don't think this is a particularly farfetched example, since
jiffies is intentionally started near the top of the range, so
slave->jiffies is likely to be high in the range after bonding is
configured at boot.

>One way to fix it would be a boolean which tracks whether or not we've
>gone past the time, and if we have then we don't bother actually
>checking the time anymore.

	It might be clearer to make the slave->jiffies some kind of
countdown instead, perhaps reusing the slave->delay used for
updelay/downdelay and eliminating slave->jiffies entirely.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com
--
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