* [PATCH net-next] bpf: restore skb->sk before pskb_trim() call
@ 2017-04-26 16:09 Eric Dumazet
2017-04-26 16:14 ` Daniel Borkmann
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Eric Dumazet @ 2017-04-26 16:09 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Andrey Konovalov, Willem de Bruijn
From: Eric Dumazet <edumazet@google•com>
While testing a fix [1] in ___pskb_trim(), addressing the WARN_ON_ONCE()
in skb_try_coalesce() reported by Andrey, I found that we had an skb
with skb->sk set but no skb->destructor.
This invalidated heuristic found in commit 158f323b9868 ("net: adjust
skb->truesize in pskb_expand_head()") and in cited patch.
Considering the BUG_ON(skb->sk) we have in skb_orphan(), we should
restrain the temporary setting to a minimal section.
[1] https://patchwork.ozlabs.org/patch/755570/
net: adjust skb->truesize in ___pskb_trim()
Fixes: 8f917bba0042 ("bpf: pass sk to helper functions")
Signed-off-by: Eric Dumazet <edumazet@google•com>
Cc: Willem de Bruijn <willemb@google•com>
Cc: Andrey Konovalov <andreyknvl@google•com>
---
net/core/filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index 9a37860a80fc78378705b681ec3b0468824cbcf4..a253a6197e6b37a7ae2fe451c646b01c861a3e22 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -98,8 +98,8 @@ int sk_filter_trim_cap(struct sock *sk, struct sk_buff *skb, unsigned int cap)
skb->sk = sk;
pkt_len = bpf_prog_run_save_cb(filter->prog, skb);
- err = pkt_len ? pskb_trim(skb, max(cap, pkt_len)) : -EPERM;
skb->sk = save_sk;
+ err = pkt_len ? pskb_trim(skb, max(cap, pkt_len)) : -EPERM;
}
rcu_read_unlock();
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net-next] bpf: restore skb->sk before pskb_trim() call
2017-04-26 16:09 [PATCH net-next] bpf: restore skb->sk before pskb_trim() call Eric Dumazet
@ 2017-04-26 16:14 ` Daniel Borkmann
2017-04-26 20:53 ` Alexei Starovoitov
2017-05-01 2:23 ` David Miller
2 siblings, 0 replies; 5+ messages in thread
From: Daniel Borkmann @ 2017-04-26 16:14 UTC (permalink / raw)
To: Eric Dumazet, David Miller; +Cc: netdev, Andrey Konovalov, Willem de Bruijn
On 04/26/2017 06:09 PM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google•com>
>
> While testing a fix [1] in ___pskb_trim(), addressing the WARN_ON_ONCE()
> in skb_try_coalesce() reported by Andrey, I found that we had an skb
> with skb->sk set but no skb->destructor.
>
> This invalidated heuristic found in commit 158f323b9868 ("net: adjust
> skb->truesize in pskb_expand_head()") and in cited patch.
>
> Considering the BUG_ON(skb->sk) we have in skb_orphan(), we should
> restrain the temporary setting to a minimal section.
>
> [1] https://patchwork.ozlabs.org/patch/755570/
> net: adjust skb->truesize in ___pskb_trim()
>
> Fixes: 8f917bba0042 ("bpf: pass sk to helper functions")
> Signed-off-by: Eric Dumazet <edumazet@google•com>
> Cc: Willem de Bruijn <willemb@google•com>
> Cc: Andrey Konovalov <andreyknvl@google•com>
Good point, thanks!
Acked-by: Daniel Borkmann <daniel@iogearbox•net>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net-next] bpf: restore skb->sk before pskb_trim() call
2017-04-26 16:09 [PATCH net-next] bpf: restore skb->sk before pskb_trim() call Eric Dumazet
2017-04-26 16:14 ` Daniel Borkmann
@ 2017-04-26 20:53 ` Alexei Starovoitov
2017-04-27 1:30 ` Willem de Bruijn
2017-05-01 2:23 ` David Miller
2 siblings, 1 reply; 5+ messages in thread
From: Alexei Starovoitov @ 2017-04-26 20:53 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Andrey Konovalov, Willem de Bruijn
On Wed, Apr 26, 2017 at 09:09:23AM -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google•com>
>
> While testing a fix [1] in ___pskb_trim(), addressing the WARN_ON_ONCE()
> in skb_try_coalesce() reported by Andrey, I found that we had an skb
> with skb->sk set but no skb->destructor.
>
> This invalidated heuristic found in commit 158f323b9868 ("net: adjust
> skb->truesize in pskb_expand_head()") and in cited patch.
>
> Considering the BUG_ON(skb->sk) we have in skb_orphan(), we should
> restrain the temporary setting to a minimal section.
>
> [1] https://patchwork.ozlabs.org/patch/755570/
> net: adjust skb->truesize in ___pskb_trim()
>
> Fixes: 8f917bba0042 ("bpf: pass sk to helper functions")
> Signed-off-by: Eric Dumazet <edumazet@google•com>
> Cc: Willem de Bruijn <willemb@google•com>
> Cc: Andrey Konovalov <andreyknvl@google•com>
Ahh. Thanks for the fix.
Acked-by: Alexei Starovoitov <ast@kernel•org>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net-next] bpf: restore skb->sk before pskb_trim() call
2017-04-26 20:53 ` Alexei Starovoitov
@ 2017-04-27 1:30 ` Willem de Bruijn
0 siblings, 0 replies; 5+ messages in thread
From: Willem de Bruijn @ 2017-04-27 1:30 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Eric Dumazet, David Miller, netdev, Andrey Konovalov
On Wed, Apr 26, 2017 at 4:53 PM, Alexei Starovoitov
<alexei.starovoitov@gmail•com> wrote:
> On Wed, Apr 26, 2017 at 09:09:23AM -0700, Eric Dumazet wrote:
>> From: Eric Dumazet <edumazet@google•com>
>>
>> While testing a fix [1] in ___pskb_trim(), addressing the WARN_ON_ONCE()
>> in skb_try_coalesce() reported by Andrey, I found that we had an skb
>> with skb->sk set but no skb->destructor.
>>
>> This invalidated heuristic found in commit 158f323b9868 ("net: adjust
>> skb->truesize in pskb_expand_head()") and in cited patch.
>>
>> Considering the BUG_ON(skb->sk) we have in skb_orphan(), we should
>> restrain the temporary setting to a minimal section.
>>
>> [1] https://patchwork.ozlabs.org/patch/755570/
>> net: adjust skb->truesize in ___pskb_trim()
>>
>> Fixes: 8f917bba0042 ("bpf: pass sk to helper functions")
>> Signed-off-by: Eric Dumazet <edumazet@google•com>
>> Cc: Willem de Bruijn <willemb@google•com>
>> Cc: Andrey Konovalov <andreyknvl@google•com>
>
> Ahh. Thanks for the fix.
> Acked-by: Alexei Starovoitov <ast@kernel•org>
Acked-by: Willem de Bruijn <willemb@google•com>
Thanks, Eric.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] bpf: restore skb->sk before pskb_trim() call
2017-04-26 16:09 [PATCH net-next] bpf: restore skb->sk before pskb_trim() call Eric Dumazet
2017-04-26 16:14 ` Daniel Borkmann
2017-04-26 20:53 ` Alexei Starovoitov
@ 2017-05-01 2:23 ` David Miller
2 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2017-05-01 2:23 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, andreyknvl, willemb
From: Eric Dumazet <eric.dumazet@gmail•com>
Date: Wed, 26 Apr 2017 09:09:23 -0700
> From: Eric Dumazet <edumazet@google•com>
>
> While testing a fix [1] in ___pskb_trim(), addressing the WARN_ON_ONCE()
> in skb_try_coalesce() reported by Andrey, I found that we had an skb
> with skb->sk set but no skb->destructor.
>
> This invalidated heuristic found in commit 158f323b9868 ("net: adjust
> skb->truesize in pskb_expand_head()") and in cited patch.
>
> Considering the BUG_ON(skb->sk) we have in skb_orphan(), we should
> restrain the temporary setting to a minimal section.
>
> [1] https://patchwork.ozlabs.org/patch/755570/
> net: adjust skb->truesize in ___pskb_trim()
>
> Fixes: 8f917bba0042 ("bpf: pass sk to helper functions")
> Signed-off-by: Eric Dumazet <edumazet@google•com>
Applied, thanks Eric.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-05-01 2:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-26 16:09 [PATCH net-next] bpf: restore skb->sk before pskb_trim() call Eric Dumazet
2017-04-26 16:14 ` Daniel Borkmann
2017-04-26 20:53 ` Alexei Starovoitov
2017-04-27 1:30 ` Willem de Bruijn
2017-05-01 2:23 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox