Message ID | 20180523182952.77006-1-willemdebruijn.kernel@gmail.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
Series | [net] ipv4: remove warning in ip_recv_error | expand |
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: Wed, 23 May 2018 14:29:52 -0400 > From: Willem de Bruijn <willemb@google.com> > > A precondition check in ip_recv_error triggered on an otherwise benign > race. Remove the warning. > > The warning triggers when passing an ipv6 socket to this ipv4 error > handling function. RaceFuzzer was able to trigger it due to a race > in setsockopt IPV6_ADDRFORM. ... > This socket option converts a v6 socket that is connected to a v4 peer > to an v4 socket. It updates the socket on the fly, changing fields in > sk as well as other structs. This is inherently non-atomic. It races > with the lockless udp_recvmsg path. > > No other code makes an assumption that these fields are updated > atomically. It is benign here, too, as ip_recv_error cares only about > the protocol of the skbs enqueued on the error queue, for which > sk_family is not a precise predictor (thanks to another isue with > IPV6_ADDRFORM). > > Link: http://lkml.kernel.org/r/20180518120826.GA19515@dragonet.kaist.ac.kr > Fixes: ("7ce875e5ecb8 ipv4: warn once on passing AF_INET6 socket to ip_recv_error") > Reported-by: DaeRyong Jeong <threeearcat@gmail.com> > Suggested-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Willem de Bruijn <willemb@google.com> Applied and queued up for -stable. The SHA1_ID doesn't go inside the (" ") of the Fixes tag, I fixed it up this time.
On Thu, May 24, 2018 at 10:18 PM, David Miller <davem@davemloft.net> wrote: > From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> > Date: Wed, 23 May 2018 14:29:52 -0400 > >> From: Willem de Bruijn <willemb@google.com> >> >> A precondition check in ip_recv_error triggered on an otherwise benign >> race. Remove the warning. >> >> The warning triggers when passing an ipv6 socket to this ipv4 error >> handling function. RaceFuzzer was able to trigger it due to a race >> in setsockopt IPV6_ADDRFORM. > ... >> This socket option converts a v6 socket that is connected to a v4 peer >> to an v4 socket. It updates the socket on the fly, changing fields in >> sk as well as other structs. This is inherently non-atomic. It races >> with the lockless udp_recvmsg path. >> >> No other code makes an assumption that these fields are updated >> atomically. It is benign here, too, as ip_recv_error cares only about >> the protocol of the skbs enqueued on the error queue, for which >> sk_family is not a precise predictor (thanks to another isue with >> IPV6_ADDRFORM). >> >> Link: http://lkml.kernel.org/r/20180518120826.GA19515@dragonet.kaist.ac.kr >> Fixes: ("7ce875e5ecb8 ipv4: warn once on passing AF_INET6 socket to ip_recv_error") >> Reported-by: DaeRyong Jeong <threeearcat@gmail.com> >> Suggested-by: Eric Dumazet <edumazet@google.com> >> Signed-off-by: Willem de Bruijn <willemb@google.com> > > Applied and queued up for -stable. > > The SHA1_ID doesn't go inside the (" ") of the Fixes tag, I fixed > it up this time. Thanks David. Sorry about that. I'll send a checkpatch.pl patch to catch such typos myself in the future. Something like +# Check format of Fixes line + if ($in_commit_log && $line =~ /^\s*fixes:/i) { + if ($line !~ /^fixes:\s[0-9a-f]{12,40}\s\(".*"\)$/i) { + WARN("BAD_FIXES", + "Fixes tag is not of form \"Fixes: <12+ chars of sha1> \(\"title line\"\)"); + } + } + though it seems that Fixes lines are expressly omitted from strict commit style checking at the moment. I don't immediately see why.
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c index 5ad2d8ed3a3f..57bbb060faaf 100644 --- a/net/ipv4/ip_sockglue.c +++ b/net/ipv4/ip_sockglue.c @@ -505,8 +505,6 @@ int ip_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len) int err; int copied; - WARN_ON_ONCE(sk->sk_family == AF_INET6); - err = -EAGAIN; skb = sock_dequeue_err_skb(sk); if (!skb)