* [PATCH net] skbuff: fix incorrect msg_zerocopy copy notifications
@ 2021-06-09 22:41 Willem de Bruijn
2021-06-10 0:17 ` Soheil Hassas Yeganeh
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Willem de Bruijn @ 2021-06-09 22:41 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, soheil, jonathan.lemon, Willem de Bruijn, Talal Ahmad
From: Willem de Bruijn <willemb@google.com>
msg_zerocopy signals if a send operation required copying with a flag
in serr->ee.ee_code.
This field can be incorrect as of the below commit, as a result of
both structs uarg and serr pointing into the same skb->cb[].
uarg->zerocopy must be read before skb->cb[] is reinitialized to hold
serr. Similar to other fields len, hi and lo, use a local variable to
temporarily hold the value.
This was not a problem before, when the value was passed as a function
argument.
Fixes: 75518851a2a0 ("skbuff: Push status and refcounts into sock_zerocopy_callback")
Reported-by: Talal Ahmad <talalahmad@google.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/core/skbuff.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 3ad22870298c..bbc3b4b62032 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1253,6 +1253,7 @@ static void __msg_zerocopy_callback(struct ubuf_info *uarg)
struct sock *sk = skb->sk;
struct sk_buff_head *q;
unsigned long flags;
+ bool is_zerocopy;
u32 lo, hi;
u16 len;
@@ -1267,6 +1268,7 @@ static void __msg_zerocopy_callback(struct ubuf_info *uarg)
len = uarg->len;
lo = uarg->id;
hi = uarg->id + len - 1;
+ is_zerocopy = uarg->zerocopy;
serr = SKB_EXT_ERR(skb);
memset(serr, 0, sizeof(*serr));
@@ -1274,7 +1276,7 @@ static void __msg_zerocopy_callback(struct ubuf_info *uarg)
serr->ee.ee_origin = SO_EE_ORIGIN_ZEROCOPY;
serr->ee.ee_data = hi;
serr->ee.ee_info = lo;
- if (!uarg->zerocopy)
+ if (!is_zerocopy)
serr->ee.ee_code |= SO_EE_CODE_ZEROCOPY_COPIED;
q = &sk->sk_error_queue;
--
2.32.0.rc1.229.g3e70b5a671-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net] skbuff: fix incorrect msg_zerocopy copy notifications
2021-06-09 22:41 [PATCH net] skbuff: fix incorrect msg_zerocopy copy notifications Willem de Bruijn
@ 2021-06-10 0:17 ` Soheil Hassas Yeganeh
2021-06-10 14:03 ` Eric Dumazet
2021-06-10 20:50 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: Soheil Hassas Yeganeh @ 2021-06-10 0:17 UTC (permalink / raw)
To: Willem de Bruijn
Cc: netdev, David Miller, Jakub Kicinski, Jonathan Lemon,
Willem de Bruijn, Talal Ahmad
On Wed, Jun 9, 2021 at 6:42 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> From: Willem de Bruijn <willemb@google.com>
>
> msg_zerocopy signals if a send operation required copying with a flag
> in serr->ee.ee_code.
>
> This field can be incorrect as of the below commit, as a result of
> both structs uarg and serr pointing into the same skb->cb[].
>
> uarg->zerocopy must be read before skb->cb[] is reinitialized to hold
> serr. Similar to other fields len, hi and lo, use a local variable to
> temporarily hold the value.
>
> This was not a problem before, when the value was passed as a function
> argument.
>
> Fixes: 75518851a2a0 ("skbuff: Push status and refcounts into sock_zerocopy_callback")
> Reported-by: Talal Ahmad <talalahmad@google.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Thank you for the fix!
> ---
> net/core/skbuff.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 3ad22870298c..bbc3b4b62032 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -1253,6 +1253,7 @@ static void __msg_zerocopy_callback(struct ubuf_info *uarg)
> struct sock *sk = skb->sk;
> struct sk_buff_head *q;
> unsigned long flags;
> + bool is_zerocopy;
> u32 lo, hi;
> u16 len;
>
> @@ -1267,6 +1268,7 @@ static void __msg_zerocopy_callback(struct ubuf_info *uarg)
> len = uarg->len;
> lo = uarg->id;
> hi = uarg->id + len - 1;
> + is_zerocopy = uarg->zerocopy;
>
> serr = SKB_EXT_ERR(skb);
> memset(serr, 0, sizeof(*serr));
> @@ -1274,7 +1276,7 @@ static void __msg_zerocopy_callback(struct ubuf_info *uarg)
> serr->ee.ee_origin = SO_EE_ORIGIN_ZEROCOPY;
> serr->ee.ee_data = hi;
> serr->ee.ee_info = lo;
> - if (!uarg->zerocopy)
> + if (!is_zerocopy)
> serr->ee.ee_code |= SO_EE_CODE_ZEROCOPY_COPIED;
>
> q = &sk->sk_error_queue;
> --
> 2.32.0.rc1.229.g3e70b5a671-goog
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] skbuff: fix incorrect msg_zerocopy copy notifications
2021-06-09 22:41 [PATCH net] skbuff: fix incorrect msg_zerocopy copy notifications Willem de Bruijn
2021-06-10 0:17 ` Soheil Hassas Yeganeh
@ 2021-06-10 14:03 ` Eric Dumazet
2021-06-10 20:50 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2021-06-10 14:03 UTC (permalink / raw)
To: Willem de Bruijn, netdev
Cc: davem, kuba, soheil, jonathan.lemon, Willem de Bruijn, Talal Ahmad
On 6/10/21 12:41 AM, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> msg_zerocopy signals if a send operation required copying with a flag
> in serr->ee.ee_code.
>
> This field can be incorrect as of the below commit, as a result of
> both structs uarg and serr pointing into the same skb->cb[].
>
> uarg->zerocopy must be read before skb->cb[] is reinitialized to hold
> serr. Similar to other fields len, hi and lo, use a local variable to
> temporarily hold the value.
>
> This was not a problem before, when the value was passed as a function
> argument.
>
> Fixes: 75518851a2a0 ("skbuff: Push status and refcounts into sock_zerocopy_callback")
> Reported-by: Talal Ahmad <talalahmad@google.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] skbuff: fix incorrect msg_zerocopy copy notifications
2021-06-09 22:41 [PATCH net] skbuff: fix incorrect msg_zerocopy copy notifications Willem de Bruijn
2021-06-10 0:17 ` Soheil Hassas Yeganeh
2021-06-10 14:03 ` Eric Dumazet
@ 2021-06-10 20:50 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-06-10 20:50 UTC (permalink / raw)
To: Willem de Bruijn
Cc: netdev, davem, kuba, soheil, jonathan.lemon, willemb, talalahmad
Hello:
This patch was applied to netdev/net.git (refs/heads/master):
On Wed, 9 Jun 2021 18:41:57 -0400 you wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> msg_zerocopy signals if a send operation required copying with a flag
> in serr->ee.ee_code.
>
> This field can be incorrect as of the below commit, as a result of
> both structs uarg and serr pointing into the same skb->cb[].
>
> [...]
Here is the summary with links:
- [net] skbuff: fix incorrect msg_zerocopy copy notifications
https://git.kernel.org/netdev/net/c/3bdd5ee0ec8c
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-10 20:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-09 22:41 [PATCH net] skbuff: fix incorrect msg_zerocopy copy notifications Willem de Bruijn
2021-06-10 0:17 ` Soheil Hassas Yeganeh
2021-06-10 14:03 ` Eric Dumazet
2021-06-10 20:50 ` patchwork-bot+netdevbpf
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.