[PATCH] netfilter: ipv4: fix NULL dereference

[PATCH] netfilter: ipv4: fix NULL dereference

[Liping Zhang]

From: Liping Zhang <liping.zhang@spreadtrum.com>

Commit fa50d974d104 ("ipv4: Namespaceify ip_default_ttl sysctl knob")
introduce the namespaceify ip_default_ttl, but sk_buff->sk maybe NULL,
so sock_net(skb->sk) will dereference the NULL pointer and oops will
happen.

Signed-off-by: Liping Zhang <liping.zhang@spreadtrum.com>
---
 net/bridge/netfilter/nft_reject_bridge.c |   20 ++++++++++----------
 net/ipv4/netfilter/ipt_SYNPROXY.c        |   16 ++++++++++------
 2 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/net/bridge/netfilter/nft_reject_bridge.c b/net/bridge/netfilter/nft_reject_bridge.c
index adc8d72..77f7e7a 100644
--- a/net/bridge/netfilter/nft_reject_bridge.c
+++ b/net/bridge/netfilter/nft_reject_bridge.c
@@ -40,7 +40,8 @@ static void nft_reject_br_push_etherhdr(struct sk_buff *oldskb,
 /* We cannot use oldskb->dev, it can be either bridge device (NF_BRIDGE INPUT)
  * or the bridge port (NF_BRIDGE PREROUTING).
  */
-static void nft_reject_br_send_v4_tcp_reset(struct sk_buff *oldskb,
+static void nft_reject_br_send_v4_tcp_reset(struct net *net,
+					    struct sk_buff *oldskb,
 					    const struct net_device *dev,
 					    int hook)
 {
@@ -48,7 +49,6 @@ static void nft_reject_br_send_v4_tcp_reset(struct sk_buff *oldskb,
 	struct iphdr *niph;
 	const struct tcphdr *oth;
 	struct tcphdr _oth;
-	struct net *net = sock_net(oldskb->sk);
 
 	if (!nft_bridge_iphdr_validate(oldskb))
 		return;
@@ -75,7 +75,8 @@ static void nft_reject_br_send_v4_tcp_reset(struct sk_buff *oldskb,
 	br_deliver(br_port_get_rcu(dev), nskb);
 }
 
-static void nft_reject_br_send_v4_unreach(struct sk_buff *oldskb,
+static void nft_reject_br_send_v4_unreach(struct net *net,
+					  struct sk_buff *oldskb,
 					  const struct net_device *dev,
 					  int hook, u8 code)
 {
@@ -86,7 +87,6 @@ static void nft_reject_br_send_v4_unreach(struct sk_buff *oldskb,
 	void *payload;
 	__wsum csum;
 	u8 proto;
-	struct net *net = sock_net(oldskb->sk);
 
 	if (oldskb->csum_bad || !nft_bridge_iphdr_validate(oldskb))
 		return;
@@ -273,17 +273,17 @@ static void nft_reject_bridge_eval(const struct nft_expr *expr,
 	case htons(ETH_P_IP):
 		switch (priv->type) {
 		case NFT_REJECT_ICMP_UNREACH:
-			nft_reject_br_send_v4_unreach(pkt->skb, pkt->in,
-						      pkt->hook,
+			nft_reject_br_send_v4_unreach(pkt->net, pkt->skb,
+						      pkt->in, pkt->hook,
 						      priv->icmp_code);
 			break;
 		case NFT_REJECT_TCP_RST:
-			nft_reject_br_send_v4_tcp_reset(pkt->skb, pkt->in,
-							pkt->hook);
+			nft_reject_br_send_v4_tcp_reset(pkt->net, pkt->skb,
+							pkt->in, pkt->hook);
 			break;
 		case NFT_REJECT_ICMPX_UNREACH:
-			nft_reject_br_send_v4_unreach(pkt->skb, pkt->in,
-						      pkt->hook,
+			nft_reject_br_send_v4_unreach(pkt->net, pkt->skb,
+						      pkt->in, pkt->hook,
 						      nft_reject_icmp_code(priv->icmp_code));
 			break;
 		}
diff --git a/net/ipv4/netfilter/ipt_SYNPROXY.c b/net/ipv4/netfilter/ipt_SYNPROXY.c
index 7b8fbb3..6b4f501 100644
--- a/net/ipv4/netfilter/ipt_SYNPROXY.c
+++ b/net/ipv4/netfilter/ipt_SYNPROXY.c
@@ -18,10 +18,10 @@
 #include <net/netfilter/nf_conntrack_synproxy.h>
 
 static struct iphdr *
-synproxy_build_ip(struct sk_buff *skb, __be32 saddr, __be32 daddr)
+synproxy_build_ip(struct net *net, struct sk_buff *skb, __be32 saddr,
+		  __be32 daddr)
 {
 	struct iphdr *iph;
-	struct net *net = sock_net(skb->sk);
 
 	skb_reset_network_header(skb);
 	iph = (struct iphdr *)skb_put(skb, sizeof(*iph));
@@ -91,7 +91,8 @@ synproxy_send_client_synack(const struct synproxy_net *snet,
 		return;
 	skb_reserve(nskb, MAX_TCP_HEADER);
 
-	niph = synproxy_build_ip(nskb, iph->daddr, iph->saddr);
+	niph = synproxy_build_ip(nf_ct_net(snet->tmpl), nskb, iph->daddr,
+				 iph->saddr);
 
 	skb_reset_transport_header(nskb);
 	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
@@ -132,7 +133,8 @@ synproxy_send_server_syn(const struct synproxy_net *snet,
 		return;
 	skb_reserve(nskb, MAX_TCP_HEADER);
 
-	niph = synproxy_build_ip(nskb, iph->saddr, iph->daddr);
+	niph = synproxy_build_ip(nf_ct_net(snet->tmpl), nskb, iph->saddr,
+				 iph->daddr);
 
 	skb_reset_transport_header(nskb);
 	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
@@ -177,7 +179,8 @@ synproxy_send_server_ack(const struct synproxy_net *snet,
 		return;
 	skb_reserve(nskb, MAX_TCP_HEADER);
 
-	niph = synproxy_build_ip(nskb, iph->daddr, iph->saddr);
+	niph = synproxy_build_ip(nf_ct_net(snet->tmpl), nskb, iph->daddr,
+				 iph->saddr);
 
 	skb_reset_transport_header(nskb);
 	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
@@ -215,7 +218,8 @@ synproxy_send_client_ack(const struct synproxy_net *snet,
 		return;
 	skb_reserve(nskb, MAX_TCP_HEADER);
 
-	niph = synproxy_build_ip(nskb, iph->saddr, iph->daddr);
+	niph = synproxy_build_ip(nf_ct_net(snet->tmpl), nskb, iph->saddr,
+				 iph->daddr);
 
 	skb_reset_transport_header(nskb);
 	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
-- 
1.7.9.5

Re: [PATCH] netfilter: ipv4: fix NULL dereference

[Nikolay Borisov]

I've been running production kernels in production with those changes
and so far I haven't observed a single crash resulting from this.
Furthermore, I  believe that all the call sites of synproxy_build_ip
should have the skb associated with a valid tcp socket, which must have
originated from a particular namespace.

On 03/23/2016 04:27 PM, Liping Zhang wrote:
> From: Liping Zhang <liping.zhang@spreadtrum.com>
> 
> Commit fa50d974d104 ("ipv4: Namespaceify ip_default_ttl sysctl knob")
> introduce the namespaceify ip_default_ttl, but sk_buff->sk maybe NULL,
> so sock_net(skb->sk) will dereference the NULL pointer and oops will
> happen.
> 
> Signed-off-by: Liping Zhang <liping.zhang@spreadtrum.com>
> ---
>  net/bridge/netfilter/nft_reject_bridge.c |   20 ++++++++++----------
>  net/ipv4/netfilter/ipt_SYNPROXY.c        |   16 ++++++++++------
>  2 files changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/net/bridge/netfilter/nft_reject_bridge.c b/net/bridge/netfilter/nft_reject_bridge.c
> index adc8d72..77f7e7a 100644
> --- a/net/bridge/netfilter/nft_reject_bridge.c
> +++ b/net/bridge/netfilter/nft_reject_bridge.c
> @@ -40,7 +40,8 @@ static void nft_reject_br_push_etherhdr(struct sk_buff *oldskb,
>  /* We cannot use oldskb->dev, it can be either bridge device (NF_BRIDGE INPUT)
>   * or the bridge port (NF_BRIDGE PREROUTING).
>   */
> -static void nft_reject_br_send_v4_tcp_reset(struct sk_buff *oldskb,
> +static void nft_reject_br_send_v4_tcp_reset(struct net *net,
> +					    struct sk_buff *oldskb,
>  					    const struct net_device *dev,
>  					    int hook)
>  {
> @@ -48,7 +49,6 @@ static void nft_reject_br_send_v4_tcp_reset(struct sk_buff *oldskb,
>  	struct iphdr *niph;
>  	const struct tcphdr *oth;
>  	struct tcphdr _oth;
> -	struct net *net = sock_net(oldskb->sk);
>  
>  	if (!nft_bridge_iphdr_validate(oldskb))
>  		return;
> @@ -75,7 +75,8 @@ static void nft_reject_br_send_v4_tcp_reset(struct sk_buff *oldskb,
>  	br_deliver(br_port_get_rcu(dev), nskb);
>  }
>  
> -static void nft_reject_br_send_v4_unreach(struct sk_buff *oldskb,
> +static void nft_reject_br_send_v4_unreach(struct net *net,
> +					  struct sk_buff *oldskb,
>  					  const struct net_device *dev,
>  					  int hook, u8 code)
>  {
> @@ -86,7 +87,6 @@ static void nft_reject_br_send_v4_unreach(struct sk_buff *oldskb,
>  	void *payload;
>  	__wsum csum;
>  	u8 proto;
> -	struct net *net = sock_net(oldskb->sk);
>  
>  	if (oldskb->csum_bad || !nft_bridge_iphdr_validate(oldskb))
>  		return;
> @@ -273,17 +273,17 @@ static void nft_reject_bridge_eval(const struct nft_expr *expr,
>  	case htons(ETH_P_IP):
>  		switch (priv->type) {
>  		case NFT_REJECT_ICMP_UNREACH:
> -			nft_reject_br_send_v4_unreach(pkt->skb, pkt->in,
> -						      pkt->hook,
> +			nft_reject_br_send_v4_unreach(pkt->net, pkt->skb,
> +						      pkt->in, pkt->hook,
>  						      priv->icmp_code);
>  			break;
>  		case NFT_REJECT_TCP_RST:
> -			nft_reject_br_send_v4_tcp_reset(pkt->skb, pkt->in,
> -							pkt->hook);
> +			nft_reject_br_send_v4_tcp_reset(pkt->net, pkt->skb,
> +							pkt->in, pkt->hook);
>  			break;
>  		case NFT_REJECT_ICMPX_UNREACH:
> -			nft_reject_br_send_v4_unreach(pkt->skb, pkt->in,
> -						      pkt->hook,
> +			nft_reject_br_send_v4_unreach(pkt->net, pkt->skb,
> +						      pkt->in, pkt->hook,
>  						      nft_reject_icmp_code(priv->icmp_code));
>  			break;
>  		}
> diff --git a/net/ipv4/netfilter/ipt_SYNPROXY.c b/net/ipv4/netfilter/ipt_SYNPROXY.c
> index 7b8fbb3..6b4f501 100644
> --- a/net/ipv4/netfilter/ipt_SYNPROXY.c
> +++ b/net/ipv4/netfilter/ipt_SYNPROXY.c
> @@ -18,10 +18,10 @@
>  #include <net/netfilter/nf_conntrack_synproxy.h>
>  
>  static struct iphdr *
> -synproxy_build_ip(struct sk_buff *skb, __be32 saddr, __be32 daddr)
> +synproxy_build_ip(struct net *net, struct sk_buff *skb, __be32 saddr,
> +		  __be32 daddr)
>  {
>  	struct iphdr *iph;
> -	struct net *net = sock_net(skb->sk);
>  
>  	skb_reset_network_header(skb);
>  	iph = (struct iphdr *)skb_put(skb, sizeof(*iph));
> @@ -91,7 +91,8 @@ synproxy_send_client_synack(const struct synproxy_net *snet,
>  		return;
>  	skb_reserve(nskb, MAX_TCP_HEADER);
>  
> -	niph = synproxy_build_ip(nskb, iph->daddr, iph->saddr);
> +	niph = synproxy_build_ip(nf_ct_net(snet->tmpl), nskb, iph->daddr,
> +				 iph->saddr);
>  
>  	skb_reset_transport_header(nskb);
>  	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
> @@ -132,7 +133,8 @@ synproxy_send_server_syn(const struct synproxy_net *snet,
>  		return;
>  	skb_reserve(nskb, MAX_TCP_HEADER);
>  
> -	niph = synproxy_build_ip(nskb, iph->saddr, iph->daddr);
> +	niph = synproxy_build_ip(nf_ct_net(snet->tmpl), nskb, iph->saddr,
> +				 iph->daddr);
>  
>  	skb_reset_transport_header(nskb);
>  	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
> @@ -177,7 +179,8 @@ synproxy_send_server_ack(const struct synproxy_net *snet,
>  		return;
>  	skb_reserve(nskb, MAX_TCP_HEADER);
>  
> -	niph = synproxy_build_ip(nskb, iph->daddr, iph->saddr);
> +	niph = synproxy_build_ip(nf_ct_net(snet->tmpl), nskb, iph->daddr,
> +				 iph->saddr);
>  
>  	skb_reset_transport_header(nskb);
>  	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
> @@ -215,7 +218,8 @@ synproxy_send_client_ack(const struct synproxy_net *snet,
>  		return;
>  	skb_reserve(nskb, MAX_TCP_HEADER);
>  
> -	niph = synproxy_build_ip(nskb, iph->saddr, iph->daddr);
> +	niph = synproxy_build_ip(nf_ct_net(snet->tmpl), nskb, iph->saddr,
> +				 iph->daddr);
>  
>  	skb_reset_transport_header(nskb);
>  	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
> 

Re: [PATCH] netfilter: ipv4: fix NULL dereference

[Nikolay Borisov]

On 03/24/2016 10:25 AM, 张李平 wrote:
> At 2016-03-24 16:00:02, "Nikolay Borisov" <kernel@kyup.com> wrote:
>> I've been running production kernels in production with those changes
>> and so far I haven't observed a single crash resulting from this.
> 
> Did you run the test with the CONFIG_NET_NS config enabled?

Of course, why else would I author the patches :)

> 
> 
>> Furthermore, I  believe that all the call sites of synproxy_build_ip
>> should have the skb associated with a valid tcp socket, which must have
> 
>> originated from a particular namespace.
> 
> 
> Actually, the sk_buff passed to synproxy_build_ip() are all alloced by
> alloc_skb, and was not associated with a specific tcp socket, so skb->sk 
> is always NULL. Further more, SYNPROXY target can be used to proxy
> the tcp connections which are not local, i.e. tcp sockets was located on
> other hosts. 

Admittedly I'm not using the synproxy module so that might explain why I
haven't seen crashes.

In any case you can add my:

Reviewed-by: Nikolay Borisov <kernel@kyup.com>

> 
> 
>>
>> On 03/23/2016 04:27 PM, Liping Zhang wrote:
>>> From: Liping Zhang <liping.zhang@spreadtrum.com>
>>>
>>> Commit fa50d974d104 ("ipv4: Namespaceify ip_default_ttl sysctl knob")
>>> introduce the namespaceify ip_default_ttl, but sk_buff->sk maybe NULL,
>>> so sock_net(skb->sk) will dereference the NULL pointer and oops will
>>> happen.
>>>
>>> Signed-off-by: Liping Zhang <liping.zhang@spreadtrum.com>
>>> ---
>>>  net/bridge/netfilter/nft_reject_bridge.c |   20 ++++++++++----------
>>>  net/ipv4/netfilter/ipt_SYNPROXY.c        |   16 ++++++++++------
>>>  2 files changed, 20 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/net/bridge/netfilter/nft_reject_bridge.c b/net/bridge/netfilter/nft_reject_bridge.c
>>> index adc8d72..77f7e7a 100644
>>> --- a/net/bridge/netfilter/nft_reject_bridge.c
>>> +++ b/net/bridge/netfilter/nft_reject_bridge.c
>>> @@ -40,7 +40,8 @@ static void nft_reject_br_push_etherhdr(struct sk_buff *oldskb,
>>>  /* We cannot use oldskb->dev, it can be either bridge device (NF_BRIDGE INPUT)
>>>   * or the bridge port (NF_BRIDGE PREROUTING).
>>>   */
>>> -static void nft_reject_br_send_v4_tcp_reset(struct sk_buff *oldskb,
>>> +static void nft_reject_br_send_v4_tcp_reset(struct net *net,
>>> +					    struct sk_buff *oldskb,
>>>  					    const struct net_device *dev,
>>>  					    int hook)
>>>  {
>>> @@ -48,7 +49,6 @@ static void nft_reject_br_send_v4_tcp_reset(struct sk_buff *oldskb,
>>>  	struct iphdr *niph;
>>>  	const struct tcphdr *oth;
>>>  	struct tcphdr _oth;
>>> -	struct net *net = sock_net(oldskb->sk);
>>>  
>>>  	if (!nft_bridge_iphdr_validate(oldskb))
>>>  		return;
>>> @@ -75,7 +75,8 @@ static void nft_reject_br_send_v4_tcp_reset(struct sk_buff *oldskb,
>>>  	br_deliver(br_port_get_rcu(dev), nskb);
>>>  }
>>>  
>>> -static void nft_reject_br_send_v4_unreach(struct sk_buff *oldskb,
>>> +static void nft_reject_br_send_v4_unreach(struct net *net,
>>> +					  struct sk_buff *oldskb,
>>>  					  const struct net_device *dev,
>>>  					  int hook, u8 code)
>>>  {
>>> @@ -86,7 +87,6 @@ static void nft_reject_br_send_v4_unreach(struct sk_buff *oldskb,
>>>  	void *payload;
>>>  	__wsum csum;
>>>  	u8 proto;
>>> -	struct net *net = sock_net(oldskb->sk);
>>>  
>>>  	if (oldskb->csum_bad || !nft_bridge_iphdr_validate(oldskb))
>>>  		return;
>>> @@ -273,17 +273,17 @@ static void nft_reject_bridge_eval(const struct nft_expr *expr,
>>>  	case htons(ETH_P_IP):
>>>  		switch (priv->type) {
>>>  		case NFT_REJECT_ICMP_UNREACH:
>>> -			nft_reject_br_send_v4_unreach(pkt->skb, pkt->in,
>>> -						      pkt->hook,
>>> +			nft_reject_br_send_v4_unreach(pkt->net, pkt->skb,
>>> +						      pkt->in, pkt->hook,
>>>  						      priv->icmp_code);
>>>  			break;
>>>  		case NFT_REJECT_TCP_RST:
>>> -			nft_reject_br_send_v4_tcp_reset(pkt->skb, pkt->in,
>>> -							pkt->hook);
>>> +			nft_reject_br_send_v4_tcp_reset(pkt->net, pkt->skb,
>>> +							pkt->in, pkt->hook);
>>>  			break;
>>>  		case NFT_REJECT_ICMPX_UNREACH:
>>> -			nft_reject_br_send_v4_unreach(pkt->skb, pkt->in,
>>> -						      pkt->hook,
>>> +			nft_reject_br_send_v4_unreach(pkt->net, pkt->skb,
>>> +						      pkt->in, pkt->hook,
>>>  						      nft_reject_icmp_code(priv->icmp_code));
>>>  			break;
>>>  		}
>>> diff --git a/net/ipv4/netfilter/ipt_SYNPROXY.c b/net/ipv4/netfilter/ipt_SYNPROXY.c
>>> index 7b8fbb3..6b4f501 100644
>>> --- a/net/ipv4/netfilter/ipt_SYNPROXY.c
>>> +++ b/net/ipv4/netfilter/ipt_SYNPROXY.c
>>> @@ -18,10 +18,10 @@
>>>  #include <net/netfilter/nf_conntrack_synproxy.h>
>>>  
>>>  static struct iphdr *
>>> -synproxy_build_ip(struct sk_buff *skb, __be32 saddr, __be32 daddr)
>>> +synproxy_build_ip(struct net *net, struct sk_buff *skb, __be32 saddr,
>>> +		  __be32 daddr)
>>>  {
>>>  	struct iphdr *iph;
>>> -	struct net *net = sock_net(skb->sk);
>>>  
>>>  	skb_reset_network_header(skb);
>>>  	iph = (struct iphdr *)skb_put(skb, sizeof(*iph));
>>> @@ -91,7 +91,8 @@ synproxy_send_client_synack(const struct synproxy_net *snet,
>>>  		return;
>>>  	skb_reserve(nskb, MAX_TCP_HEADER);
>>>  
>>> -	niph = synproxy_build_ip(nskb, iph->daddr, iph->saddr);
>>> +	niph = synproxy_build_ip(nf_ct_net(snet->tmpl), nskb, iph->daddr,
>>> +				 iph->saddr);
>>>  
>>>  	skb_reset_transport_header(nskb);
>>>  	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
>>> @@ -132,7 +133,8 @@ synproxy_send_server_syn(const struct synproxy_net *snet,
>>>  		return;
>>>  	skb_reserve(nskb, MAX_TCP_HEADER);
>>>  
>>> -	niph = synproxy_build_ip(nskb, iph->saddr, iph->daddr);
>>> +	niph = synproxy_build_ip(nf_ct_net(snet->tmpl), nskb, iph->saddr,
>>> +				 iph->daddr);
>>>  
>>>  	skb_reset_transport_header(nskb);
>>>  	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
>>> @@ -177,7 +179,8 @@ synproxy_send_server_ack(const struct synproxy_net *snet,
>>>  		return;
>>>  	skb_reserve(nskb, MAX_TCP_HEADER);
>>>  
>>> -	niph = synproxy_build_ip(nskb, iph->daddr, iph->saddr);
>>> +	niph = synproxy_build_ip(nf_ct_net(snet->tmpl), nskb, iph->daddr,
>>> +				 iph->saddr);
>>>  
>>>  	skb_reset_transport_header(nskb);
>>>  	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
>>> @@ -215,7 +218,8 @@ synproxy_send_client_ack(const struct synproxy_net *snet,
>>>  		return;
>>>  	skb_reserve(nskb, MAX_TCP_HEADER);
>>>  
>>> -	niph = synproxy_build_ip(nskb, iph->saddr, iph->daddr);
>>> +	niph = synproxy_build_ip(nf_ct_net(snet->tmpl), nskb, iph->saddr,
>>> +				 iph->daddr);
>>>  
>>>  	skb_reset_transport_header(nskb);
>>>  	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
>>>
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: ipv4: fix NULL dereference

[Pablo Neira Ayuso]

On Thu, Mar 24, 2016 at 10:00:02AM +0200, Nikolay Borisov wrote:
> I've been running production kernels in production with those changes
> and so far I haven't observed a single crash resulting from this.
> Furthermore, I  believe that all the call sites of synproxy_build_ip
> should have the skb associated with a valid tcp socket, which must have
> originated from a particular namespace.

Please, always Cc: netfilter-devel@vger.kernel.org for patches that
modify netfilter code. Your change is buggy, we cannot assume skb->sk
set on packets that are being forwarded, we could have detected this
following this process.

Thanks.

Re: [PATCH] netfilter: ipv4: fix NULL dereference

[Pablo Neira Ayuso]

On Wed, Mar 23, 2016 at 10:27:30PM +0800, Liping Zhang wrote:
> diff --git a/net/ipv4/netfilter/ipt_SYNPROXY.c b/net/ipv4/netfilter/ipt_SYNPROXY.c
> index 7b8fbb3..6b4f501 100644
> --- a/net/ipv4/netfilter/ipt_SYNPROXY.c
> +++ b/net/ipv4/netfilter/ipt_SYNPROXY.c
> @@ -18,10 +18,10 @@
>  #include <net/netfilter/nf_conntrack_synproxy.h>
>  
>  static struct iphdr *
> -synproxy_build_ip(struct sk_buff *skb, __be32 saddr, __be32 daddr)
> +synproxy_build_ip(struct net *net, struct sk_buff *skb, __be32 saddr,
> +		  __be32 daddr)
>  {
>  	struct iphdr *iph;
> -	struct net *net = sock_net(skb->sk);
>  
>  	skb_reset_network_header(skb);
>  	iph = (struct iphdr *)skb_put(skb, sizeof(*iph));
> @@ -91,7 +91,8 @@ synproxy_send_client_synack(const struct synproxy_net *snet,
>  		return;
>  	skb_reserve(nskb, MAX_TCP_HEADER);
>  
> -	niph = synproxy_build_ip(nskb, iph->daddr, iph->saddr);
> +	niph = synproxy_build_ip(nf_ct_net(snet->tmpl), nskb, iph->daddr,
> +				 iph->saddr);
>  
>  	skb_reset_transport_header(nskb);
>  	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
> @@ -132,7 +133,8 @@ synproxy_send_server_syn(const struct synproxy_net *snet,
>  		return;
>  	skb_reserve(nskb, MAX_TCP_HEADER);
>  
> -	niph = synproxy_build_ip(nskb, iph->saddr, iph->daddr);
> +	niph = synproxy_build_ip(nf_ct_net(snet->tmpl), nskb, iph->saddr,
> +				 iph->daddr);

Could you also pass net as parameter to synproxy_send_server_syn() ?

par->net provides this from synproxy_tg4().

Thanks.

Re:Re: [PATCH] netfilter: ipv4: fix NULL dereference

[Liping Zhang]

At 2016-03-25 04:22:05, "Pablo Neira Ayuso" <pablo@netfilter.org> wrote:
>
>Could you also pass net as parameter to synproxy_send_server_syn() ?
>
>par->net provides this from synproxy_tg4().

Not pass the net but replace the first parameter 'snet' with 'net' seems better?
snet is only used in synproxy_recv_client_ack->synproxy_send_server_syn call path,
and in other call path, actually we only need net,  and we can call synproxy_pernet(net)
to get the snet.

like follows:
-synproxy_send_server_syn(const struct synproxy_net *snet,
+synproxy_send_server_syn(struct net *net,

-synproxy_recv_client_ack(const struct synproxy_net *snet,
+synproxy_recv_client_ack(struct net *net,
                         const struct sk_buff *skb, const struct tcphdr *th,
                         struct synproxy_options *opts, u32 recv_seq)
 {
+       struct synproxy_net *snet = synproxy_pernet(net);

>
>Thanks.

Re: Re: [PATCH] netfilter: ipv4: fix NULL dereference

[Pablo Neira Ayuso]

On Fri, Mar 25, 2016 at 02:38:15PM +0800, Liping Zhang wrote:
> At 2016-03-25 04:22:05, "Pablo Neira Ayuso" <pablo@netfilter.org> wrote:
> >
> >Could you also pass net as parameter to synproxy_send_server_syn() ?
> >
> >par->net provides this from synproxy_tg4().
> 
> Not pass the net but replace the first parameter 'snet' with 'net' seems better?
> snet is only used in synproxy_recv_client_ack->synproxy_send_server_syn call path,
> and in other call path, actually we only need net,  and we can call synproxy_pernet(net)
> to get the snet.
> 
> like follows:
> -synproxy_send_server_syn(const struct synproxy_net *snet,
> +synproxy_send_server_syn(struct net *net,
> 
> -synproxy_recv_client_ack(const struct synproxy_net *snet,
> +synproxy_recv_client_ack(struct net *net,
>                          const struct sk_buff *skb, const struct tcphdr *th,
>                          struct synproxy_options *opts, u32 recv_seq)
>  {
> +       struct synproxy_net *snet = synproxy_pernet(net);

Fine with me.

Re: [PATCH] netfilter: ipv4: fix NULL dereference

[Liping Zhang]

At 2016-03-25 04:22:05, "Pablo Neira Ayuso" <pablo@netfilter.org> wrote:
>
>Could you also pass net as parameter to synproxy_send_server_syn() ?
>
>par->net provides this from synproxy_tg4().

Not pass the net but replace the first parameter 'snet' with 'net' seems better?
snet is only used in synproxy_recv_client_ack->synproxy_send_server_syn call path,
and in other call path, actually we only need net,  and we can call synproxy_pernet(net)
to get the snet.

like follows:
-synproxy_send_server_syn(const struct synproxy_net *snet,
+synproxy_send_server_syn(struct net *net,

-synproxy_recv_client_ack(const struct synproxy_net *snet,
+synproxy_recv_client_ack(struct net *net,
                         const struct sk_buff *skb, const struct tcphdr *th,
                         struct synproxy_options *opts, u32 recv_seq)  {
+       struct synproxy_net *snet = synproxy_pernet(net);

>
>Thanks.