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

2016-06-03 Thread David Miller 
From: Eric Dumazet 
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.
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

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

2016-06-03 Thread David Miller 
From: Eric Dumazet 
Date: Thu, 02 Jun 2016 14:52:43 -0700

> From: Eric Dumazet 
> 
> 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 
> Reported-by: Paul Moore 
> Tested-by: Paul Moore 
> Tested-by: Stephen Smalley 
> Cc: samanthakumar 

Applied, thanks Eric.
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

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

2016-06-03 Thread Eric Dumazet 
From: Eric Dumazet 

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 
Reported-by: Paul Moore 
Tested-by: Paul Moore 
Tested-by: Stephen Smalley 
Cc: samanthakumar 
---
 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)) {


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

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

2016-06-03 Thread Eric Dumazet 
On Thu, 2016-06-02 at 17:36 -0400, Paul Moore wrote:
> On Wed, Jun 1, 2016 at 4:44 PM, Stephen Smalley  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 
> >>>   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 
> >>>Signed-off-by: Willem de Bruijn 
> >>>Signed-off-by: David S. Miller 
> >>>
> >>> ... 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 
> 
> 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 

I am going to send the official patch right away, thanks !


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

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

2016-06-02 Thread Paul Moore 
On Wed, Jun 1, 2016 at 4:44 PM, Stephen Smalley  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 
>>>   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 
>>>Signed-off-by: Willem de Bruijn 
>>>Signed-off-by: David S. Miller 
>>>
>>> ... 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 

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 

>> 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
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

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

2016-06-01 Thread Stephen Smalley 
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 
>>   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 
>>Signed-off-by: Willem de Bruijn 
>>Signed-off-by: David S. Miller 
>>
>> ... 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 

> 
> 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-le...@tycho.nsa.gov.
> To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
> 

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

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

2016-06-01 Thread Eric Dumazet 
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 
>   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 
>Signed-off-by: Willem de Bruijn 
>Signed-off-by: David S. Miller 
> 
> ... 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)) {


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

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

2016-06-01 Thread Paul Moore 
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 
  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 
   Signed-off-by: Willem de Bruijn 
   Signed-off-by: David S. Miller 

... 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
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.