Re: [PATCH net] ipvs: use skb_to_full_sk() helper

[David Miller]

From: Eric Dumazet 
Date: Thu, 12 Nov 2015 09:14:12 -0800

> From: Eric Dumazet 
> 
> SYNACK packets might be attached to request sockets.
> 
> Use skb_to_full_sk() helper to avoid illegal accesses to
> inet_sk(skb->sk)
> 
> Fixes: ca6fb0651883 ("tcp: attach SYNACK messages to request sockets instead 
> of listener")
> Signed-off-by: Eric Dumazet 
> Reported-by: Sander Eikelenboom 

I'll apply this directly, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] ipvs: use skb_to_full_sk() helper

[Julian Anastasov]

Hello,

On Thu, 12 Nov 2015, Eric Dumazet wrote:

> From: Eric Dumazet 
> 
> SYNACK packets might be attached to request sockets.
> 
> Use skb_to_full_sk() helper to avoid illegal accesses to
> inet_sk(skb->sk)
> 
> Fixes: ca6fb0651883 ("tcp: attach SYNACK messages to request sockets instead 
> of listener")
> Signed-off-by: Eric Dumazet 
> Reported-by: Sander Eikelenboom 

Looks good, I guess Simon will take the patch for
the ipvs tree.

Acked-by: Julian Anastasov 

> ---
>  net/netfilter/ipvs/ip_vs_core.c |   16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index 1e24fff53e4b..f57b4dcdb233 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -1176,6 +1176,7 @@ ip_vs_out(struct netns_ipvs *ipvs, unsigned int 
> hooknum, struct sk_buff *skb, in
>   struct ip_vs_protocol *pp;
>   struct ip_vs_proto_data *pd;
>   struct ip_vs_conn *cp;
> + struct sock *sk;
>  
>   EnterFunction(11);
>  
> @@ -1183,13 +1184,12 @@ ip_vs_out(struct netns_ipvs *ipvs, unsigned int 
> hooknum, struct sk_buff *skb, in
>   if (skb->ipvs_property)
>   return NF_ACCEPT;
>  
> + sk = skb_to_full_sk(skb);
>   /* Bad... Do not break raw sockets */
> - if (unlikely(skb->sk != NULL && hooknum == NF_INET_LOCAL_OUT &&
> + if (unlikely(sk && hooknum == NF_INET_LOCAL_OUT &&
>af == AF_INET)) {
> - struct sock *sk = skb->sk;
> - struct inet_sock *inet = inet_sk(skb->sk);
>  
> - if (inet && sk->sk_family == PF_INET && inet->nodefrag)
> + if (sk->sk_family == PF_INET && inet_sk(sk)->nodefrag)
>   return NF_ACCEPT;
>   }
>  
> @@ -1681,6 +1681,7 @@ ip_vs_in(struct netns_ipvs *ipvs, unsigned int hooknum, 
> struct sk_buff *skb, int
>   struct ip_vs_conn *cp;
>   int ret, pkts;
>   int conn_reuse_mode;
> + struct sock *sk;
>  
>   /* Already marked as IPVS request or reply? */
>   if (skb->ipvs_property)
> @@ -1708,12 +1709,11 @@ ip_vs_in(struct netns_ipvs *ipvs, unsigned int 
> hooknum, struct sk_buff *skb, int
>   ip_vs_fill_iph_skb(af, skb, false, );
>  
>   /* Bad... Do not break raw sockets */
> - if (unlikely(skb->sk != NULL && hooknum == NF_INET_LOCAL_OUT &&
> + sk = skb_to_full_sk(skb);
> + if (unlikely(sk && hooknum == NF_INET_LOCAL_OUT &&
>af == AF_INET)) {
> - struct sock *sk = skb->sk;
> - struct inet_sock *inet = inet_sk(skb->sk);
>  
> - if (inet && sk->sk_family == PF_INET && inet->nodefrag)
> + if (sk->sk_family == PF_INET && inet_sk(sk)->nodefrag)
>   return NF_ACCEPT;
>   }

Regards

--
Julian Anastasov 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] ipvs: use skb_to_full_sk() helper

[Simon Horman]

[Attn Dave]
[Cc Pablo]

On Sat, Nov 14, 2015 at 01:37:46PM +0200, Julian Anastasov wrote:
> 
>   Hello,
> 
> On Thu, 12 Nov 2015, Eric Dumazet wrote:
> 
> > From: Eric Dumazet 
> > 
> > SYNACK packets might be attached to request sockets.
> > 
> > Use skb_to_full_sk() helper to avoid illegal accesses to
> > inet_sk(skb->sk)
> > 
> > Fixes: ca6fb0651883 ("tcp: attach SYNACK messages to request sockets 
> > instead of listener")
> > Signed-off-by: Eric Dumazet 
> > Reported-by: Sander Eikelenboom 
> 
>   Looks good, I guess Simon will take the patch for
> the ipvs tree.
> 
> Acked-by: Julian Anastasov 

Dave,

It looks like this patch has a compile-time dependency on
54abc686c2d1 ("net: add skb_to_full_sk() helper and use it in
selinux_netlbl_skbuff_setsid()") which is currently present in
net but not nf. The latter tree is the usual path for IPVS fixes.

With the above in mind I think it would be easiest if you could
pick this patch up directly and add it to net with:

Acked-by: Simon Horman 

An alternative would be for Pablo to merge net into nf and
for me to then prepare a pull request for him. But it seems a bit
excessive for what otherwise appears to be a straightforward patch.

> > ---
> >  net/netfilter/ipvs/ip_vs_core.c |   16 
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/net/netfilter/ipvs/ip_vs_core.c 
> > b/net/netfilter/ipvs/ip_vs_core.c
> > index 1e24fff53e4b..f57b4dcdb233 100644
> > --- a/net/netfilter/ipvs/ip_vs_core.c
> > +++ b/net/netfilter/ipvs/ip_vs_core.c
> > @@ -1176,6 +1176,7 @@ ip_vs_out(struct netns_ipvs *ipvs, unsigned int 
> > hooknum, struct sk_buff *skb, in
> > struct ip_vs_protocol *pp;
> > struct ip_vs_proto_data *pd;
> > struct ip_vs_conn *cp;
> > +   struct sock *sk;
> >  
> > EnterFunction(11);
> >  
> > @@ -1183,13 +1184,12 @@ ip_vs_out(struct netns_ipvs *ipvs, unsigned int 
> > hooknum, struct sk_buff *skb, in
> > if (skb->ipvs_property)
> > return NF_ACCEPT;
> >  
> > +   sk = skb_to_full_sk(skb);
> > /* Bad... Do not break raw sockets */
> > -   if (unlikely(skb->sk != NULL && hooknum == NF_INET_LOCAL_OUT &&
> > +   if (unlikely(sk && hooknum == NF_INET_LOCAL_OUT &&
> >  af == AF_INET)) {
> > -   struct sock *sk = skb->sk;
> > -   struct inet_sock *inet = inet_sk(skb->sk);
> >  
> > -   if (inet && sk->sk_family == PF_INET && inet->nodefrag)
> > +   if (sk->sk_family == PF_INET && inet_sk(sk)->nodefrag)
> > return NF_ACCEPT;
> > }
> >  
> > @@ -1681,6 +1681,7 @@ ip_vs_in(struct netns_ipvs *ipvs, unsigned int 
> > hooknum, struct sk_buff *skb, int
> > struct ip_vs_conn *cp;
> > int ret, pkts;
> > int conn_reuse_mode;
> > +   struct sock *sk;
> >  
> > /* Already marked as IPVS request or reply? */
> > if (skb->ipvs_property)
> > @@ -1708,12 +1709,11 @@ ip_vs_in(struct netns_ipvs *ipvs, unsigned int 
> > hooknum, struct sk_buff *skb, int
> > ip_vs_fill_iph_skb(af, skb, false, );
> >  
> > /* Bad... Do not break raw sockets */
> > -   if (unlikely(skb->sk != NULL && hooknum == NF_INET_LOCAL_OUT &&
> > +   sk = skb_to_full_sk(skb);
> > +   if (unlikely(sk && hooknum == NF_INET_LOCAL_OUT &&
> >  af == AF_INET)) {
> > -   struct sock *sk = skb->sk;
> > -   struct inet_sock *inet = inet_sk(skb->sk);
> >  
> > -   if (inet && sk->sk_family == PF_INET && inet->nodefrag)
> > +   if (sk->sk_family == PF_INET && inet_sk(sk)->nodefrag)
> > return NF_ACCEPT;
> > }
> 
> Regards
> 
> --
> Julian Anastasov 
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH net] ipvs: use skb_to_full_sk() helper

[Eric Dumazet]

From: Eric Dumazet 

SYNACK packets might be attached to request sockets.

Use skb_to_full_sk() helper to avoid illegal accesses to
inet_sk(skb->sk)

Fixes: ca6fb0651883 ("tcp: attach SYNACK messages to request sockets instead of 
listener")
Signed-off-by: Eric Dumazet 
Reported-by: Sander Eikelenboom 
---
 net/netfilter/ipvs/ip_vs_core.c |   16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index 1e24fff53e4b..f57b4dcdb233 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1176,6 +1176,7 @@ ip_vs_out(struct netns_ipvs *ipvs, unsigned int hooknum, 
struct sk_buff *skb, in
struct ip_vs_protocol *pp;
struct ip_vs_proto_data *pd;
struct ip_vs_conn *cp;
+   struct sock *sk;
 
EnterFunction(11);
 
@@ -1183,13 +1184,12 @@ ip_vs_out(struct netns_ipvs *ipvs, unsigned int 
hooknum, struct sk_buff *skb, in
if (skb->ipvs_property)
return NF_ACCEPT;
 
+   sk = skb_to_full_sk(skb);
/* Bad... Do not break raw sockets */
-   if (unlikely(skb->sk != NULL && hooknum == NF_INET_LOCAL_OUT &&
+   if (unlikely(sk && hooknum == NF_INET_LOCAL_OUT &&
 af == AF_INET)) {
-   struct sock *sk = skb->sk;
-   struct inet_sock *inet = inet_sk(skb->sk);
 
-   if (inet && sk->sk_family == PF_INET && inet->nodefrag)
+   if (sk->sk_family == PF_INET && inet_sk(sk)->nodefrag)
return NF_ACCEPT;
}
 
@@ -1681,6 +1681,7 @@ ip_vs_in(struct netns_ipvs *ipvs, unsigned int hooknum, 
struct sk_buff *skb, int
struct ip_vs_conn *cp;
int ret, pkts;
int conn_reuse_mode;
+   struct sock *sk;
 
/* Already marked as IPVS request or reply? */
if (skb->ipvs_property)
@@ -1708,12 +1709,11 @@ ip_vs_in(struct netns_ipvs *ipvs, unsigned int hooknum, 
struct sk_buff *skb, int
ip_vs_fill_iph_skb(af, skb, false, );
 
/* Bad... Do not break raw sockets */
-   if (unlikely(skb->sk != NULL && hooknum == NF_INET_LOCAL_OUT &&
+   sk = skb_to_full_sk(skb);
+   if (unlikely(sk && hooknum == NF_INET_LOCAL_OUT &&
 af == AF_INET)) {
-   struct sock *sk = skb->sk;
-   struct inet_sock *inet = inet_sk(skb->sk);
 
-   if (inet && sk->sk_family == PF_INET && inet->nodefrag)
+   if (sk->sk_family == PF_INET && inet_sk(sk)->nodefrag)
return NF_ACCEPT;
}
 


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html