* [PATCH 1/1] net: bridge: fix a bug on using a neighbour cache entry without checking its state
@ 2019-01-06 3:28 kchen
2019-01-07 17:10 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: kchen @ 2019-01-06 3:28 UTC (permalink / raw)
To: nikolay; +Cc: netdev, linux-kernel, JianJhen Chen
From: JianJhen Chen <kchen@synology.com>
When handling DNAT'ed packets on a bridge device, the neighbour cache entry
from lookup was used without checking its state. It means that a cache entry
in the NUD_STALE state will be used directly instead of entering the NUD_DELAY
state to confirm the reachability of the neighbor.
This problem becomes worse after commit 2724680bceee ("neigh: Keep neighbour
cache entries if number of them is small enough."), since all neighbour cache
entries in the NUD_STALE state will be kept in the neighbour table as long as
the number of cache entries does not exceed the value specified in gc_thresh1.
This commit validates the state of a neighbour cache entry before using
the entry.
Signed-off-by: JianJhen Chen <kchen@synology.com>
Reviewed-by: JinLin Chen <jlchen@synology.com>
---
net/bridge/br_netfilter_hooks.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index b1b5e85..ed683e5 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -278,7 +278,7 @@ int br_nf_pre_routing_finish_bridge(struct net *net, struct sock *sk, struct sk_
struct nf_bridge_info *nf_bridge = nf_bridge_info_get(skb);
int ret;
- if (neigh->hh.hh_len) {
+ if ((neigh->nud_state & NUD_CONNECTED) && neigh->hh.hh_len) {
neigh_hh_bridge(&neigh->hh, skb);
skb->dev = nf_bridge->physindev;
ret = br_handle_frame_finish(net, sk, skb);
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] net: bridge: fix a bug on using a neighbour cache entry without checking its state
2019-01-06 3:28 [PATCH 1/1] net: bridge: fix a bug on using a neighbour cache entry without checking its state kchen
@ 2019-01-07 17:10 ` David Miller
2019-01-08 6:04 ` kchen
0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2019-01-07 17:10 UTC (permalink / raw)
To: kchen; +Cc: nikolay, netdev, linux-kernel
From: kchen <kchen@synology.com>
Date: Sun, 6 Jan 2019 11:28:13 +0800
> From: JianJhen Chen <kchen@synology.com>
>
> When handling DNAT'ed packets on a bridge device, the neighbour cache entry
> from lookup was used without checking its state. It means that a cache entry
> in the NUD_STALE state will be used directly instead of entering the NUD_DELAY
> state to confirm the reachability of the neighbor.
>
> This problem becomes worse after commit 2724680bceee ("neigh: Keep neighbour
> cache entries if number of them is small enough."), since all neighbour cache
> entries in the NUD_STALE state will be kept in the neighbour table as long as
> the number of cache entries does not exceed the value specified in gc_thresh1.
>
> This commit validates the state of a neighbour cache entry before using
> the entry.
>
> Signed-off-by: JianJhen Chen <kchen@synology.com>
> Reviewed-by: JinLin Chen <jlchen@synology.com>
Indeed, this looks correct.
Applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] net: bridge: fix a bug on using a neighbour cache entry without checking its state
2019-01-07 17:10 ` David Miller
@ 2019-01-08 6:04 ` kchen
0 siblings, 0 replies; 3+ messages in thread
From: kchen @ 2019-01-08 6:04 UTC (permalink / raw)
To: David Miller; +Cc: nikolay, netdev, linux-kernel, JianJhen Chen, linux-kernel
From: David Miller <davem@davemloft.net>
Date: Mon, 07 Jan 2019 09:10:31 -0800
> From: David Miller <davem@davemloft.net>
>
> From: kchen <kchen@synology.com>
> Date: Sun, 6 Jan 2019 11:28:13 +0800
>
>> From: JianJhen Chen <kchen@synology.com>
>>
>> When handling DNAT'ed packets on a bridge device, the neighbour cache entry
>> from lookup was used without checking its state. It means that a cache entry
>> in the NUD_STALE state will be used directly instead of entering the NUD_DELAY
>> state to confirm the reachability of the neighbor.
>>
>> This problem becomes worse after commit 2724680bceee ("neigh: Keep neighbour
>> cache entries if number of them is small enough."), since all neighbour cache
>> entries in the NUD_STALE state will be kept in the neighbour table as long as
>> the number of cache entries does not exceed the value specified in gc_thresh1.
>>
>> This commit validates the state of a neighbour cache entry before using
>> the entry.
>>
>> Signed-off-by: JianJhen Chen <kchen@synology.com>
>> Reviewed-by: JinLin Chen <jlchen@synology.com>
>
> Indeed, this looks correct.
>
> Applied and queued up for -stable, thanks.
>
Thank you for the confirmation.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-01-08 6:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-06 3:28 [PATCH 1/1] net: bridge: fix a bug on using a neighbour cache entry without checking its state kchen
2019-01-07 17:10 ` David Miller
2019-01-08 6:04 ` kchen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.