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: <20160920174649.GN1843@nanopsycho.orion>
Date:   Tue, 20 Sep 2016 19:46:49 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     netdev@...r.kernel.org
Subject: Re: UBSAN reports issue in ip_idents_reserve

Tue, Sep 20, 2016 at 04:18:12PM CEST, eric.dumazet@...il.com wrote:
>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);
> 

This patch makes ubsan silent.


>
>