| Message ID | 20210609224157.1800831-1-willemdebruijn.kernel@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 3bdd5ee0ec8c14131d560da492e6df452c6fdd75 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] skbuff: fix incorrect msg_zerocopy copy notifications | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 5 maintainers not CCed: linmiaohe@huawei.com gnault@redhat.com cong.wang@bytedance.com linyunsheng@huawei.com alobakin@pm.me |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 1 this patch: 1 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 22 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 1 this patch: 1 |
| netdev/header_inline | success | Link |
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 >
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>
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
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;