* [PATCH net] xps: must clear sender_cpu before forwarding [not found] ` <CANn89iLHkYc2Kay7z+v=tHMW5aYSqUf8V4f70Z-d5fUuuUXnpw@mail.gmail.com> @ 2015-03-12 1:42 ` Eric Dumazet 2015-03-12 3:51 ` David Miller 2015-03-12 5:21 ` John 0 siblings, 2 replies; 6+ messages in thread From: Eric Dumazet @ 2015-03-12 1:42 UTC (permalink / raw) To: John, David Miller; +Cc: netdev From: Eric Dumazet <edumazet@google.com> John reported that my previous commit added a regression on his router. This is because sender_cpu & napi_id share a common location, so get_xps_queue() can see garbage and perform an out of bound access. We need to make sure sender_cpu is cleared before doing the transmit, otherwise any NIC busy poll enabled (skb_mark_napi_id()) can trigger this bug. Signed-off-by: Eric Dumazet <edumazet@google.com> Reported-by: John <jw@nuclearfallout.net> Bisected-by: John <jw@nuclearfallout.net> Fixes: 2bd82484bb4c ("xps: fix xps for stacked devices") --- include/linux/skbuff.h | 7 +++++++ net/core/skbuff.c | 2 +- net/ipv4/ip_forward.c | 1 + net/ipv6/ip6_output.c | 1 + 4 files changed, 10 insertions(+), 1 deletion(-) diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 30007afe70b3..f54d6659713a 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -948,6 +948,13 @@ static inline void skb_copy_hash(struct sk_buff *to, const struct sk_buff *from) to->l4_hash = from->l4_hash; }; +static inline void skb_sender_cpu_clear(struct sk_buff *skb) +{ +#ifdef CONFIG_XPS + skb->sender_cpu = 0; +#endif +} + #ifdef NET_SKBUFF_DATA_USES_OFFSET static inline unsigned char *skb_end_pointer(const struct sk_buff *skb) { diff --git a/net/core/skbuff.c b/net/core/skbuff.c index f80507823531..434e78e5254d 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -4173,7 +4173,7 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet) skb->ignore_df = 0; skb_dst_drop(skb); skb->mark = 0; - skb->sender_cpu = 0; + skb_sender_cpu_clear(skb); skb_init_secmark(skb); secpath_reset(skb); nf_reset(skb); diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c index 787b3c294ce6..d9bc28ac5d1b 100644 --- a/net/ipv4/ip_forward.c +++ b/net/ipv4/ip_forward.c @@ -67,6 +67,7 @@ static int ip_forward_finish(struct sk_buff *skb) if (unlikely(opt->optlen)) ip_forward_options(skb); + skb_sender_cpu_clear(skb); return dst_output(skb); } diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c index 0a04a37305d5..7e80b61b51ff 100644 --- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c @@ -318,6 +318,7 @@ static int ip6_forward_proxy_check(struct sk_buff *skb) static inline int ip6_forward_finish(struct sk_buff *skb) { + skb_sender_cpu_clear(skb); return dst_output(skb); } ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net] xps: must clear sender_cpu before forwarding 2015-03-12 1:42 ` [PATCH net] xps: must clear sender_cpu before forwarding Eric Dumazet @ 2015-03-12 3:51 ` David Miller 2015-03-13 2:36 ` John 2015-03-12 5:21 ` John 1 sibling, 1 reply; 6+ messages in thread From: David Miller @ 2015-03-12 3:51 UTC (permalink / raw) To: eric.dumazet; +Cc: jw, netdev From: Eric Dumazet <eric.dumazet@gmail.com> Date: Wed, 11 Mar 2015 18:42:02 -0700 > From: Eric Dumazet <edumazet@google.com> > > John reported that my previous commit added a regression > on his router. > > This is because sender_cpu & napi_id share a common location, > so get_xps_queue() can see garbage and perform an out of bound access. > > We need to make sure sender_cpu is cleared before doing the transmit, > otherwise any NIC busy poll enabled (skb_mark_napi_id()) can trigger > this bug. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Reported-by: John <jw@nuclearfallout.net> > Bisected-by: John <jw@nuclearfallout.net> > Fixes: 2bd82484bb4c ("xps: fix xps for stacked devices") Applied, thanks Eric. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] xps: must clear sender_cpu before forwarding 2015-03-12 3:51 ` David Miller @ 2015-03-13 2:36 ` John 2015-03-13 2:53 ` David Miller 2015-03-13 2:57 ` Eric Dumazet 0 siblings, 2 replies; 6+ messages in thread From: John @ 2015-03-13 2:36 UTC (permalink / raw) To: David Miller, eric.dumazet; +Cc: netdev On 3/11/2015 8:51 PM, David Miller wrote: > From: Eric Dumazet <eric.dumazet@gmail.com> > Date: Wed, 11 Mar 2015 18:42:02 -0700 > >> From: Eric Dumazet <edumazet@google.com> >> >> John reported that my previous commit added a regression >> on his router. >> >> This is because sender_cpu & napi_id share a common location, >> so get_xps_queue() can see garbage and perform an out of bound access. >> >> We need to make sure sender_cpu is cleared before doing the transmit, >> otherwise any NIC busy poll enabled (skb_mark_napi_id()) can trigger >> this bug. >> >> Signed-off-by: Eric Dumazet <edumazet@google.com> >> Reported-by: John <jw@nuclearfallout.net> >> Bisected-by: John <jw@nuclearfallout.net> >> Fixes: 2bd82484bb4c ("xps: fix xps for stacked devices") > Applied, thanks Eric. Running this patch, I have confirmed that I no longer see panics under the conditions that I saw them previously. -John ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] xps: must clear sender_cpu before forwarding 2015-03-13 2:36 ` John @ 2015-03-13 2:53 ` David Miller 2015-03-13 2:57 ` Eric Dumazet 1 sibling, 0 replies; 6+ messages in thread From: David Miller @ 2015-03-13 2:53 UTC (permalink / raw) To: jw; +Cc: eric.dumazet, netdev From: John <jw@nuclearfallout.net> Date: Thu, 12 Mar 2015 19:36:38 -0700 > Running this patch, I have confirmed that I no longer see panics under > the conditions that I saw them previously. Thanks for testing. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] xps: must clear sender_cpu before forwarding 2015-03-13 2:36 ` John 2015-03-13 2:53 ` David Miller @ 2015-03-13 2:57 ` Eric Dumazet 1 sibling, 0 replies; 6+ messages in thread From: Eric Dumazet @ 2015-03-13 2:57 UTC (permalink / raw) To: John; +Cc: David Miller, netdev On Thu, 2015-03-12 at 19:36 -0700, John wrote: > Running this patch, I have confirmed that I no longer see panics under > the conditions that I saw them previously. Thanks a lot John ! ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] xps: must clear sender_cpu before forwarding 2015-03-12 1:42 ` [PATCH net] xps: must clear sender_cpu before forwarding Eric Dumazet 2015-03-12 3:51 ` David Miller @ 2015-03-12 5:21 ` John 1 sibling, 0 replies; 6+ messages in thread From: John @ 2015-03-12 5:21 UTC (permalink / raw) To: Eric Dumazet, David Miller; +Cc: netdev Thanks, Eric. I will test again tomorrow. -John On 3/11/2015 6:42 PM, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > John reported that my previous commit added a regression > on his router. > > This is because sender_cpu & napi_id share a common location, > so get_xps_queue() can see garbage and perform an out of bound access. > > We need to make sure sender_cpu is cleared before doing the transmit, > otherwise any NIC busy poll enabled (skb_mark_napi_id()) can trigger > this bug. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Reported-by: John <jw@nuclearfallout.net> > Bisected-by: John <jw@nuclearfallout.net> > Fixes: 2bd82484bb4c ("xps: fix xps for stacked devices") > --- > include/linux/skbuff.h | 7 +++++++ > net/core/skbuff.c | 2 +- > net/ipv4/ip_forward.c | 1 + > net/ipv6/ip6_output.c | 1 + > 4 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index 30007afe70b3..f54d6659713a 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h > @@ -948,6 +948,13 @@ static inline void skb_copy_hash(struct sk_buff *to, const struct sk_buff *from) > to->l4_hash = from->l4_hash; > }; > > +static inline void skb_sender_cpu_clear(struct sk_buff *skb) > +{ > +#ifdef CONFIG_XPS > + skb->sender_cpu = 0; > +#endif > +} > + > #ifdef NET_SKBUFF_DATA_USES_OFFSET > static inline unsigned char *skb_end_pointer(const struct sk_buff *skb) > { > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index f80507823531..434e78e5254d 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -4173,7 +4173,7 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet) > skb->ignore_df = 0; > skb_dst_drop(skb); > skb->mark = 0; > - skb->sender_cpu = 0; > + skb_sender_cpu_clear(skb); > skb_init_secmark(skb); > secpath_reset(skb); > nf_reset(skb); > diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c > index 787b3c294ce6..d9bc28ac5d1b 100644 > --- a/net/ipv4/ip_forward.c > +++ b/net/ipv4/ip_forward.c > @@ -67,6 +67,7 @@ static int ip_forward_finish(struct sk_buff *skb) > if (unlikely(opt->optlen)) > ip_forward_options(skb); > > + skb_sender_cpu_clear(skb); > return dst_output(skb); > } > > diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c > index 0a04a37305d5..7e80b61b51ff 100644 > --- a/net/ipv6/ip6_output.c > +++ b/net/ipv6/ip6_output.c > @@ -318,6 +318,7 @@ static int ip6_forward_proxy_check(struct sk_buff *skb) > > static inline int ip6_forward_finish(struct sk_buff *skb) > { > + skb_sender_cpu_clear(skb); > return dst_output(skb); > } > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-03-13 2:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <5500E52C.7080603@nuclearfallout.net>
[not found] ` <CANn89iLHkYc2Kay7z+v=tHMW5aYSqUf8V4f70Z-d5fUuuUXnpw@mail.gmail.com>
2015-03-12 1:42 ` [PATCH net] xps: must clear sender_cpu before forwarding Eric Dumazet
2015-03-12 3:51 ` David Miller
2015-03-13 2:36 ` John
2015-03-13 2:53 ` David Miller
2015-03-13 2:57 ` Eric Dumazet
2015-03-12 5:21 ` John
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.