* [PATCH] fib_trie: remove potential out of bound access
@ 2013-08-05 18:18 Eric Dumazet
2013-08-05 22:27 ` David Miller
2013-08-05 22:41 ` Stephen Hemminger
0 siblings, 2 replies; 4+ messages in thread
From: Eric Dumazet @ 2013-08-05 18:18 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Andrey Konovalov, Dmitry Vyukov
From: Eric Dumazet <edumazet@google.com>
AddressSanitizer [1] dynamic checker pointed a potential
out of bound access in leaf_walk_rcu()
We could allocate one more slot in tnode_new() to leave the prefetch()
in-place but it looks not worth the pain.
Bug added in commit 82cfbb008572b ("[IPV4] fib_trie: iterator recode")
[1] :
https://code.google.com/p/address-sanitizer/wiki/AddressSanitizerForKernel
Reported-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
---
net/ipv4/fib_trie.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 108a1e9c..3df6d3e 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -71,7 +71,6 @@
#include <linux/init.h>
#include <linux/list.h>
#include <linux/slab.h>
-#include <linux/prefetch.h>
#include <linux/export.h>
#include <net/net_namespace.h>
#include <net/ip.h>
@@ -1761,10 +1760,8 @@ static struct leaf *leaf_walk_rcu(struct tnode *p, struct rt_trie_node *c)
if (!c)
continue;
- if (IS_LEAF(c)) {
- prefetch(rcu_dereference_rtnl(p->child[idx]));
+ if (IS_LEAF(c))
return (struct leaf *) c;
- }
/* Rescan start scanning in new node */
p = (struct tnode *) c;
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] fib_trie: remove potential out of bound access
2013-08-05 18:18 [PATCH] fib_trie: remove potential out of bound access Eric Dumazet
@ 2013-08-05 22:27 ` David Miller
2013-08-05 22:41 ` Stephen Hemminger
1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2013-08-05 22:27 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, andreyknvl, dvyukov
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 05 Aug 2013 11:18:49 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> AddressSanitizer [1] dynamic checker pointed a potential
> out of bound access in leaf_walk_rcu()
>
> We could allocate one more slot in tnode_new() to leave the prefetch()
> in-place but it looks not worth the pain.
>
> Bug added in commit 82cfbb008572b ("[IPV4] fib_trie: iterator recode")
>
> [1] :
> https://code.google.com/p/address-sanitizer/wiki/AddressSanitizerForKernel
>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
I question the validity of the prefetch anyways, even without the
out-of-bounds concerns.
Applied and queued up for -stable, thanks Eric.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fib_trie: remove potential out of bound access
2013-08-05 18:18 [PATCH] fib_trie: remove potential out of bound access Eric Dumazet
2013-08-05 22:27 ` David Miller
@ 2013-08-05 22:41 ` Stephen Hemminger
2013-08-05 23:00 ` Eric Dumazet
1 sibling, 1 reply; 4+ messages in thread
From: Stephen Hemminger @ 2013-08-05 22:41 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Andrey Konovalov, Dmitry Vyukov
On Mon, 05 Aug 2013 11:18:49 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> AddressSanitizer [1] dynamic checker pointed a potential
> out of bound access in leaf_walk_rcu()
>
> We could allocate one more slot in tnode_new() to leave the prefetch()
> in-place but it looks not worth the pain.
>
> Bug added in commit 82cfbb008572b ("[IPV4] fib_trie: iterator recode")
>
> [1] :
> https://code.google.com/p/address-sanitizer/wiki/AddressSanitizerForKernel
>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
Isn't prefetch supposed to always be safe, even out of bounds; even prefetch(NULL).
Although I really doubt prefetch helps in in this code anyway.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fib_trie: remove potential out of bound access
2013-08-05 22:41 ` Stephen Hemminger
@ 2013-08-05 23:00 ` Eric Dumazet
0 siblings, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2013-08-05 23:00 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev, Andrey Konovalov, Dmitry Vyukov
On Mon, 2013-08-05 at 15:41 -0700, Stephen Hemminger wrote:
> On Mon, 05 Aug 2013 11:18:49 -0700
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> > From: Eric Dumazet <edumazet@google.com>
> >
> > AddressSanitizer [1] dynamic checker pointed a potential
> > out of bound access in leaf_walk_rcu()
> >
> > We could allocate one more slot in tnode_new() to leave the prefetch()
> > in-place but it looks not worth the pain.
> >
> > Bug added in commit 82cfbb008572b ("[IPV4] fib_trie: iterator recode")
> >
> > [1] :
> > https://code.google.com/p/address-sanitizer/wiki/AddressSanitizerForKernel
> >
> > Reported-by: Andrey Konovalov <andreyknvl@google.com>
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Cc: Dmitry Vyukov <dvyukov@google.com>
>
> Isn't prefetch supposed to always be safe, even out of bounds; even prefetch(NULL).
> Although I really doubt prefetch helps in in this code anyway.
prefetch(...) was not the problem here.
The problem was X = array[N] with N being >= size(array)
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-08-05 23:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-05 18:18 [PATCH] fib_trie: remove potential out of bound access Eric Dumazet
2013-08-05 22:27 ` David Miller
2013-08-05 22:41 ` Stephen Hemminger
2013-08-05 23:00 ` Eric Dumazet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).