diff mbox

Possible problem with e6afc8ac ("udp: remove headers from UDP packets before queueing")

Message ID 1464904363.5939.185.camel@edumazet-glaptop3.roam.corp.google.com (mailing list archive)
State Accepted
Headers show

Commit Message

Eric Dumazet June 2, 2016, 9:52 p.m. UTC 
From: Eric Dumazet <edumazet@google.com>

Paul Moore tracked a regression caused by a recent commit, which
mistakenly assumed that sk_filter() could be avoided if socket
had no current BPF filter.

The intent was to avoid udp_lib_checksum_complete() overhead.

But sk_filter() also checks skb_pfmemalloc() and
security_sock_rcv_skb(), so better call it.

Fixes: e6afc8ace6dd ("udp: remove headers from UDP packets before queueing")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Paul Moore <paul@paul-moore.com>
Tested-by: Paul Moore <paul@paul-moore.com>
Tested-by: Stephen Smalley <sds@tycho.nsa.gov>
Cc: samanthakumar <samanthakumar@google.com>
---
 net/ipv4/udp.c |   10 +++++-----
 net/ipv6/udp.c |   12 ++++++------
 2 files changed, 11 insertions(+), 11 deletions(-)

Comments

David Miller June 2, 2016, 10:31 p.m. UTC | #1 
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 02 Jun 2016 14:52:43 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> Paul Moore tracked a regression caused by a recent commit, which
> mistakenly assumed that sk_filter() could be avoided if socket
> had no current BPF filter.
> 
> The intent was to avoid udp_lib_checksum_complete() overhead.
> 
> But sk_filter() also checks skb_pfmemalloc() and
> security_sock_rcv_skb(), so better call it.
> 
> Fixes: e6afc8ace6dd ("udp: remove headers from UDP packets before queueing")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Paul Moore <paul@paul-moore.com>
> Tested-by: Paul Moore <paul@paul-moore.com>
> Tested-by: Stephen Smalley <sds@tycho.nsa.gov>
> Cc: samanthakumar <samanthakumar@google.com>

Applied, thanks Eric.
Eric Dumazet June 3, 2016, 2:58 a.m. UTC | #2 
On Thu, 2016-06-02 at 18:31 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 02 Jun 2016 14:52:43 -0700
> 
> > From: Eric Dumazet <edumazet@google.com>
> > 
> > Paul Moore tracked a regression caused by a recent commit, which
> > mistakenly assumed that sk_filter() could be avoided if socket
> > had no current BPF filter.
> > 
> > The intent was to avoid udp_lib_checksum_complete() overhead.
> > 
> > But sk_filter() also checks skb_pfmemalloc() and
> > security_sock_rcv_skb(), so better call it.
> > 
> > Fixes: e6afc8ace6dd ("udp: remove headers from UDP packets before queueing")
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Reported-by: Paul Moore <paul@paul-moore.com>
> > Tested-by: Paul Moore <paul@paul-moore.com>
> > Tested-by: Stephen Smalley <sds@tycho.nsa.gov>
> > Cc: samanthakumar <samanthakumar@google.com>
> 
> Applied, thanks Eric.

Arg, I totally messed up the patch title :(
David Miller June 3, 2016, 4:23 a.m. UTC | #3 
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 02 Jun 2016 19:58:26 -0700

> Arg, I totally messed up the patch title :(

I noticed it was odd, but it's not a big deal.
diff mbox

Patch

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index d56c0559b477..0ff31d97d485 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1618,12 +1618,12 @@  int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 		}
 	}
 
-	if (rcu_access_pointer(sk->sk_filter)) {
-		if (udp_lib_checksum_complete(skb))
+	if (rcu_access_pointer(sk->sk_filter) &&
+	    udp_lib_checksum_complete(skb))
 			goto csum_error;
-		if (sk_filter(sk, skb))
-			goto drop;
-	}
+
+	if (sk_filter(sk, skb))
+		goto drop;
 
 	udp_csum_pull_header(skb);
 	if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) {
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 2da1896af934..f421c9f23c5b 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -653,12 +653,12 @@  int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 		}
 	}
 
-	if (rcu_access_pointer(sk->sk_filter)) {
-		if (udp_lib_checksum_complete(skb))
-			goto csum_error;
-		if (sk_filter(sk, skb))
-			goto drop;
-	}
+	if (rcu_access_pointer(sk->sk_filter) &&
+	    udp_lib_checksum_complete(skb))
+		goto csum_error;
+
+	if (sk_filter(sk, skb))
+		goto drop;
 
 	udp_csum_pull_header(skb);
 	if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) {