| Message ID | 20201126151220.2819322-1-willemdebruijn.kernel@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] sock: set sk_err to ee_errno on dequeue from errq | 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/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, 8 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 1 this patch: 1 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Thu, Nov 26, 2020 at 10:12 AM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > From: Willem de Bruijn <willemb@google.com> > > When setting sk_err, set it to ee_errno, not ee_origin. > > Commit f5f99309fa74 ("sock: do not set sk_err in > sock_dequeue_err_skb") disabled updating sk_err on errq dequeue, > which is correct for most error types (origins): > > - sk->sk_err = err; > > Commit 38b257938ac6 ("sock: reset sk_err when the error queue is > empty") reenabled the behavior for IMCP origins, which do require it: > > + if (icmp_next) > + sk->sk_err = SKB_EXT_ERR(skb_next)->ee.ee_origin; > > But read from ee_errno. > > Fixes: 38b257938ac6 ("sock: reset sk_err when the error queue is empty") > Reported-by: Ayush Ranjan <ayushranjan@google.com> > Signed-off-by: Willem de Bruijn <willemb@google.com> Acked-by: Soheil Hassas Yeganeh <soheil@google.com> Thank you for catching this! > --- > net/core/skbuff.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 1ba8f0163744..06c526e0d810 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -4549,7 +4549,7 @@ struct sk_buff *sock_dequeue_err_skb(struct sock *sk) > if (skb && (skb_next = skb_peek(q))) { > icmp_next = is_icmp_err_skb(skb_next); > if (icmp_next) > - sk->sk_err = SKB_EXT_ERR(skb_next)->ee.ee_origin; > + sk->sk_err = SKB_EXT_ERR(skb_next)->ee.ee_errno; > } > spin_unlock_irqrestore(&q->lock, flags); > > -- > 2.29.2.454.gaff20da3a2-goog >
On Thu, 26 Nov 2020 12:28:36 -0500 Soheil Hassas Yeganeh wrote: > > From: Willem de Bruijn <willemb@google.com> > > > > When setting sk_err, set it to ee_errno, not ee_origin. > > > > Commit f5f99309fa74 ("sock: do not set sk_err in > > sock_dequeue_err_skb") disabled updating sk_err on errq dequeue, > > which is correct for most error types (origins): > > > > - sk->sk_err = err; > > > > Commit 38b257938ac6 ("sock: reset sk_err when the error queue is > > empty") reenabled the behavior for IMCP origins, which do require it: > > > > + if (icmp_next) > > + sk->sk_err = SKB_EXT_ERR(skb_next)->ee.ee_origin; > > > > But read from ee_errno. > > > > Fixes: 38b257938ac6 ("sock: reset sk_err when the error queue is empty") > > Reported-by: Ayush Ranjan <ayushranjan@google.com> > > Signed-off-by: Willem de Bruijn <willemb@google.com> > > Acked-by: Soheil Hassas Yeganeh <soheil@google.com> Applied, thanks!
diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 1ba8f0163744..06c526e0d810 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -4549,7 +4549,7 @@ struct sk_buff *sock_dequeue_err_skb(struct sock *sk) if (skb && (skb_next = skb_peek(q))) { icmp_next = is_icmp_err_skb(skb_next); if (icmp_next) - sk->sk_err = SKB_EXT_ERR(skb_next)->ee.ee_origin; + sk->sk_err = SKB_EXT_ERR(skb_next)->ee.ee_errno; } spin_unlock_irqrestore(&q->lock, flags);