netdev - Re: UBSAN reports issue in ip_idents

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: <1474381092.23058.10.camel@edumazet-glaptop3.roam.corp.google.com>
Date:   Tue, 20 Sep 2016 07:18:12 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     netdev@...r.kernel.org
Subject: Re: UBSAN reports issue in ip_idents_reserve

On Tue, 2016-09-20 at 07:11 -0700, Eric Dumazet wrote:
> On Tue, 2016-09-20 at 15:39 +0200, Jiri Pirko wrote:
> 
> > I see. So how to silent the warning?
> > 
> 
> We can replace the atomic_add_return() and use a loop around
> atomic_read() and atomic_cmpxhg()
> 
> This would change the nice property of x86 xadd into a loop.
> 
> Or we also could fallback to random generation if the atomic_cmpxchg()
> fails.
> 
> I'll provide a patch, thanks.
> 

Could you try the following ?

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index
b52496fd51075821c39435f50ac62f813967aecc..91dc108ef6dc75df80f0e73b6fa062d98dc9a58a 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -476,12 +476,19 @@ u32 ip_idents_reserve(u32 hash, int segs)
 	atomic_t *p_id = ip_idents + hash % IP_IDENTS_SZ;
 	u32 old = ACCESS_ONCE(*p_tstamp);
 	u32 now = (u32)jiffies;
-	u32 delta = 0;
+	u32 new, delta = 0;
 
 	if (old != now && cmpxchg(p_tstamp, old, now) == old)
 		delta = prandom_u32_max(now - old);
 
-	return atomic_add_return(segs + delta, p_id) - segs;
+	old = (u32)atomic_read(p_id);
+	new = old + delta + segs;
+	/* Do not try too hard, if multiple cpus are there,
+	 * just fallback to pseudo random number.
+	 */
+	if (unlikely(atomic_cmpxchg(p_id, old, new) != old))
+		new = prandom_u32();
+	return new;
 }
 EXPORT_SYMBOL(ip_idents_reserve);