* Possible problem with e6afc8ac ("udp: remove headers from UDP packets before queueing")
@ 2016-06-01 19:01 Paul Moore
2016-06-01 19:18 ` Eric Dumazet
0 siblings, 1 reply; 10+ messages in thread
From: Paul Moore @ 2016-06-01 19:01 UTC (permalink / raw)
To: samanthakumar; +Cc: netdev, linux-security-module, selinux
Hello,
I'm currently trying to debug a problem with 4.7-rc1 and labeled
networking over UDP. I'm having some difficulty with the latest
4.7-rc1 builds on my test system at the moment so I haven't been able
to concisely identify the problem, but looking through the commits in
4.7-rc1 I think there may be a problem with the following:
commit e6afc8ace6dd5cef5e812f26c72579da8806f5ac
Author: samanthakumar <samanthakumar@google.com>
Date: Tue Apr 5 12:41:15 2016 -0400
udp: remove headers from UDP packets before queueing
Remove UDP transport headers before queueing packets for reception.
This change simplifies a follow-up patch to add MSG_PEEK support.
Signed-off-by: Sam Kumar <samanthakumar@google.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
... it appears that this commit changes things so that sk_filter() is
only called when sk->sk_filter is not NULL. While this is fine for
the traditional socket filter case, it causes problems with LSMs that
make use of security_sock_rcv_skb() to enforce per-packet access
controls.
Hopefully I'll get 4.7-rc1 booting soon and I can do a proper
bisection test around this patch, but I wanted to mention this now in
case others are seeing the same problem.
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Possible problem with e6afc8ac ("udp: remove headers from UDP packets before queueing")
2016-06-01 19:01 Possible problem with e6afc8ac ("udp: remove headers from UDP packets before queueing") Paul Moore
@ 2016-06-01 19:18 ` Eric Dumazet
[not found] ` <1464808695.5939.167.camel-XN9IlZ5yJG9HTL0Zs8A6p+yfmBU6pStAUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Eric Dumazet @ 2016-06-01 19:18 UTC (permalink / raw)
To: Paul Moore; +Cc: samanthakumar, netdev, linux-security-module, selinux
On Wed, 2016-06-01 at 15:01 -0400, Paul Moore wrote:
> Hello,
>
> I'm currently trying to debug a problem with 4.7-rc1 and labeled
> networking over UDP. I'm having some difficulty with the latest
> 4.7-rc1 builds on my test system at the moment so I haven't been able
> to concisely identify the problem, but looking through the commits in
> 4.7-rc1 I think there may be a problem with the following:
>
> commit e6afc8ace6dd5cef5e812f26c72579da8806f5ac
> Author: samanthakumar <samanthakumar@google.com>
> Date: Tue Apr 5 12:41:15 2016 -0400
>
> udp: remove headers from UDP packets before queueing
>
> Remove UDP transport headers before queueing packets for reception.
> This change simplifies a follow-up patch to add MSG_PEEK support.
>
> Signed-off-by: Sam Kumar <samanthakumar@google.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> ... it appears that this commit changes things so that sk_filter() is
> only called when sk->sk_filter is not NULL. While this is fine for
> the traditional socket filter case, it causes problems with LSMs that
> make use of security_sock_rcv_skb() to enforce per-packet access
> controls.
>
> Hopefully I'll get 4.7-rc1 booting soon and I can do a proper
> bisection test around this patch, but I wanted to mention this now in
> case others are seeing the same problem.
>
Thanks for the report. Please try following fix.
sk_filter() got additional features like the skb_pfmemalloc() things and
security_sock_rcv_skb()
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)) {
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: Possible problem with e6afc8ac ("udp: remove headers from UDP packets before queueing")
2016-06-01 19:18 ` Eric Dumazet
@ 2016-06-01 20:44 ` Stephen Smalley
0 siblings, 0 replies; 10+ messages in thread
From: Stephen Smalley @ 2016-06-01 20:44 UTC (permalink / raw)
To: Eric Dumazet, Paul Moore
Cc: samanthakumar-hpIqsD4AKlfQT0dZR+AlfA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
selinux-+05T5uksL2qpZYMLLGbcSA, netdev-u79uwXL29TY76Z2rM5mHXA
On 06/01/2016 03:18 PM, Eric Dumazet wrote:
> On Wed, 2016-06-01 at 15:01 -0400, Paul Moore wrote:
>> Hello,
>>
>> I'm currently trying to debug a problem with 4.7-rc1 and labeled
>> networking over UDP. I'm having some difficulty with the latest
>> 4.7-rc1 builds on my test system at the moment so I haven't been able
>> to concisely identify the problem, but looking through the commits in
>> 4.7-rc1 I think there may be a problem with the following:
>>
>> commit e6afc8ace6dd5cef5e812f26c72579da8806f5ac
>> Author: samanthakumar <samanthakumar-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>> Date: Tue Apr 5 12:41:15 2016 -0400
>>
>> udp: remove headers from UDP packets before queueing
>>
>> Remove UDP transport headers before queueing packets for reception.
>> This change simplifies a follow-up patch to add MSG_PEEK support.
>>
>> Signed-off-by: Sam Kumar <samanthakumar-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Willem de Bruijn <willemb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
>>
>> ... it appears that this commit changes things so that sk_filter() is
>> only called when sk->sk_filter is not NULL. While this is fine for
>> the traditional socket filter case, it causes problems with LSMs that
>> make use of security_sock_rcv_skb() to enforce per-packet access
>> controls.
>>
>> Hopefully I'll get 4.7-rc1 booting soon and I can do a proper
>> bisection test around this patch, but I wanted to mention this now in
>> case others are seeing the same problem.
>>
>
> Thanks for the report. Please try following fix.
>
> sk_filter() got additional features like the skb_pfmemalloc() things and
> security_sock_rcv_skb()
This resolved the SELinux regression for me.
Tested-by: Stephen Smalley <sds-+05T5uksL2qpZYMLLGbcSA@public.gmane.org>
>
> 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)) {
>
>
> _______________________________________________
> Selinux mailing list
> Selinux-+05T5uksL2qpZYMLLGbcSA@public.gmane.org
> To unsubscribe, send email to Selinux-leave-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
> To get help, send an email containing "help" to Selinux-request-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
>
_______________________________________________
Selinux mailing list
Selinux-+05T5uksL2qpZYMLLGbcSA@public.gmane.org
To unsubscribe, send email to Selinux-leave-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
To get help, send an email containing "help" to Selinux-request-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Possible problem with e6afc8ac ("udp: remove headers from UDP packets before queueing")
@ 2016-06-01 20:44 ` Stephen Smalley
0 siblings, 0 replies; 10+ messages in thread
From: Stephen Smalley @ 2016-06-01 20:44 UTC (permalink / raw)
To: Eric Dumazet, Paul Moore
Cc: samanthakumar, linux-security-module, selinux, netdev
On 06/01/2016 03:18 PM, Eric Dumazet wrote:
> On Wed, 2016-06-01 at 15:01 -0400, Paul Moore wrote:
>> Hello,
>>
>> I'm currently trying to debug a problem with 4.7-rc1 and labeled
>> networking over UDP. I'm having some difficulty with the latest
>> 4.7-rc1 builds on my test system at the moment so I haven't been able
>> to concisely identify the problem, but looking through the commits in
>> 4.7-rc1 I think there may be a problem with the following:
>>
>> commit e6afc8ace6dd5cef5e812f26c72579da8806f5ac
>> Author: samanthakumar <samanthakumar@google.com>
>> Date: Tue Apr 5 12:41:15 2016 -0400
>>
>> udp: remove headers from UDP packets before queueing
>>
>> Remove UDP transport headers before queueing packets for reception.
>> This change simplifies a follow-up patch to add MSG_PEEK support.
>>
>> Signed-off-by: Sam Kumar <samanthakumar@google.com>
>> Signed-off-by: Willem de Bruijn <willemb@google.com>
>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>
>> ... it appears that this commit changes things so that sk_filter() is
>> only called when sk->sk_filter is not NULL. While this is fine for
>> the traditional socket filter case, it causes problems with LSMs that
>> make use of security_sock_rcv_skb() to enforce per-packet access
>> controls.
>>
>> Hopefully I'll get 4.7-rc1 booting soon and I can do a proper
>> bisection test around this patch, but I wanted to mention this now in
>> case others are seeing the same problem.
>>
>
> Thanks for the report. Please try following fix.
>
> sk_filter() got additional features like the skb_pfmemalloc() things and
> security_sock_rcv_skb()
This resolved the SELinux regression for me.
Tested-by: Stephen Smalley <sds@tycho.nsa.gov>
>
> 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)) {
>
>
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Possible problem with e6afc8ac ("udp: remove headers from UDP packets before queueing")
2016-06-01 20:44 ` Stephen Smalley
(?)
@ 2016-06-02 21:36 ` Paul Moore
2016-06-02 21:44 ` Eric Dumazet
2016-06-02 21:52 ` Eric Dumazet
-1 siblings, 2 replies; 10+ messages in thread
From: Paul Moore @ 2016-06-02 21:36 UTC (permalink / raw)
To: Eric Dumazet
Cc: samanthakumar, linux-security-module, selinux, netdev, Stephen Smalley
On Wed, Jun 1, 2016 at 4:44 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On 06/01/2016 03:18 PM, Eric Dumazet wrote:
>> On Wed, 2016-06-01 at 15:01 -0400, Paul Moore wrote:
>>> Hello,
>>>
>>> I'm currently trying to debug a problem with 4.7-rc1 and labeled
>>> networking over UDP. I'm having some difficulty with the latest
>>> 4.7-rc1 builds on my test system at the moment so I haven't been able
>>> to concisely identify the problem, but looking through the commits in
>>> 4.7-rc1 I think there may be a problem with the following:
>>>
>>> commit e6afc8ace6dd5cef5e812f26c72579da8806f5ac
>>> Author: samanthakumar <samanthakumar@google.com>
>>> Date: Tue Apr 5 12:41:15 2016 -0400
>>>
>>> udp: remove headers from UDP packets before queueing
>>>
>>> Remove UDP transport headers before queueing packets for reception.
>>> This change simplifies a follow-up patch to add MSG_PEEK support.
>>>
>>> Signed-off-by: Sam Kumar <samanthakumar@google.com>
>>> Signed-off-by: Willem de Bruijn <willemb@google.com>
>>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>>
>>> ... it appears that this commit changes things so that sk_filter() is
>>> only called when sk->sk_filter is not NULL. While this is fine for
>>> the traditional socket filter case, it causes problems with LSMs that
>>> make use of security_sock_rcv_skb() to enforce per-packet access
>>> controls.
>>>
>>> Hopefully I'll get 4.7-rc1 booting soon and I can do a proper
>>> bisection test around this patch, but I wanted to mention this now in
>>> case others are seeing the same problem.
>>
>> Thanks for the report. Please try following fix.
>>
>> sk_filter() got additional features like the skb_pfmemalloc() things and
>> security_sock_rcv_skb()
>
> This resolved the SELinux regression for me.
>
> Tested-by: Stephen Smalley <sds@tycho.nsa.gov>
The patch works for me too. Eric, are you going to send this to DaveM
(assuming he isn't listening in on this thread and picking it up
himself)?
Tested-by: Paul Moore <paul@paul-moore.com>
>> 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)) {
>>
>>
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Possible problem with e6afc8ac ("udp: remove headers from UDP packets before queueing")
2016-06-02 21:36 ` Paul Moore
@ 2016-06-02 21:44 ` Eric Dumazet
2016-06-02 21:52 ` Eric Dumazet
1 sibling, 0 replies; 10+ messages in thread
From: Eric Dumazet @ 2016-06-02 21:44 UTC (permalink / raw)
To: Paul Moore
Cc: samanthakumar, linux-security-module, selinux, netdev, Stephen Smalley
On Thu, 2016-06-02 at 17:36 -0400, Paul Moore wrote:
> On Wed, Jun 1, 2016 at 4:44 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> > On 06/01/2016 03:18 PM, Eric Dumazet wrote:
> >> On Wed, 2016-06-01 at 15:01 -0400, Paul Moore wrote:
> >>> Hello,
> >>>
> >>> I'm currently trying to debug a problem with 4.7-rc1 and labeled
> >>> networking over UDP. I'm having some difficulty with the latest
> >>> 4.7-rc1 builds on my test system at the moment so I haven't been able
> >>> to concisely identify the problem, but looking through the commits in
> >>> 4.7-rc1 I think there may be a problem with the following:
> >>>
> >>> commit e6afc8ace6dd5cef5e812f26c72579da8806f5ac
> >>> Author: samanthakumar <samanthakumar@google.com>
> >>> Date: Tue Apr 5 12:41:15 2016 -0400
> >>>
> >>> udp: remove headers from UDP packets before queueing
> >>>
> >>> Remove UDP transport headers before queueing packets for reception.
> >>> This change simplifies a follow-up patch to add MSG_PEEK support.
> >>>
> >>> Signed-off-by: Sam Kumar <samanthakumar@google.com>
> >>> Signed-off-by: Willem de Bruijn <willemb@google.com>
> >>> Signed-off-by: David S. Miller <davem@davemloft.net>
> >>>
> >>> ... it appears that this commit changes things so that sk_filter() is
> >>> only called when sk->sk_filter is not NULL. While this is fine for
> >>> the traditional socket filter case, it causes problems with LSMs that
> >>> make use of security_sock_rcv_skb() to enforce per-packet access
> >>> controls.
> >>>
> >>> Hopefully I'll get 4.7-rc1 booting soon and I can do a proper
> >>> bisection test around this patch, but I wanted to mention this now in
> >>> case others are seeing the same problem.
> >>
> >> Thanks for the report. Please try following fix.
> >>
> >> sk_filter() got additional features like the skb_pfmemalloc() things and
> >> security_sock_rcv_skb()
> >
> > This resolved the SELinux regression for me.
> >
> > Tested-by: Stephen Smalley <sds@tycho.nsa.gov>
>
> The patch works for me too. Eric, are you going to send this to DaveM
> (assuming he isn't listening in on this thread and picking it up
> himself)?
>
> Tested-by: Paul Moore <paul@paul-moore.com>
I am going to send the official patch right away, thanks !
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Possible problem with e6afc8ac ("udp: remove headers from UDP packets before queueing")
2016-06-02 21:36 ` Paul Moore
2016-06-02 21:44 ` Eric Dumazet
@ 2016-06-02 21:52 ` Eric Dumazet
2016-06-02 22:31 ` David Miller
1 sibling, 1 reply; 10+ messages in thread
From: Eric Dumazet @ 2016-06-02 21:52 UTC (permalink / raw)
To: Paul Moore, David Miller
Cc: samanthakumar, linux-security-module, selinux, netdev,
Stephen Smalley, samanthakumar
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(-)
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)) {
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: Possible problem with e6afc8ac ("udp: remove headers from UDP packets before queueing")
2016-06-02 21:52 ` Eric Dumazet
@ 2016-06-02 22:31 ` David Miller
2016-06-03 2:58 ` Eric Dumazet
0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2016-06-02 22:31 UTC (permalink / raw)
To: eric.dumazet
Cc: paul, samanthakumar, linux-security-module, selinux, netdev, sds
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Possible problem with e6afc8ac ("udp: remove headers from UDP packets before queueing")
2016-06-02 22:31 ` David Miller
@ 2016-06-03 2:58 ` Eric Dumazet
2016-06-03 4:23 ` David Miller
0 siblings, 1 reply; 10+ messages in thread
From: Eric Dumazet @ 2016-06-03 2:58 UTC (permalink / raw)
To: David Miller
Cc: paul, samanthakumar, linux-security-module, selinux, netdev, sds
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 :(
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Possible problem with e6afc8ac ("udp: remove headers from UDP packets before queueing")
2016-06-03 2:58 ` Eric Dumazet
@ 2016-06-03 4:23 ` David Miller
0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2016-06-03 4:23 UTC (permalink / raw)
To: eric.dumazet
Cc: paul, samanthakumar, linux-security-module, selinux, netdev, sds
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-06-03 4:23 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-01 19:01 Possible problem with e6afc8ac ("udp: remove headers from UDP packets before queueing") Paul Moore
2016-06-01 19:18 ` Eric Dumazet
[not found] ` <1464808695.5939.167.camel-XN9IlZ5yJG9HTL0Zs8A6p+yfmBU6pStAUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
2016-06-01 20:44 ` Stephen Smalley
2016-06-01 20:44 ` Stephen Smalley
2016-06-02 21:36 ` Paul Moore
2016-06-02 21:44 ` Eric Dumazet
2016-06-02 21:52 ` Eric Dumazet
2016-06-02 22:31 ` David Miller
2016-06-03 2:58 ` Eric Dumazet
2016-06-03 4:23 ` David Miller
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.