[PATCH nf 1/1] netfilter: nat_masquerade: Check oom when invoke nfct_nat

[PATCH nf 1/1] netfilter: nat_masquerade: Check oom when invoke nfct_nat

[fgao]

From: Gao Feng <fgao@ikuai8.com>

When memory is exhausted, nf_ct_nat_ext_add may return NULL. Then
nf_nat_ipv4_fn and nf_nat_ipv6_fn would return NF_ACCEPT in this
case.
So we need add the NULL check when invoke nfct_nat in these two
functions.

Signed-off-by: Gao Feng <fgao@ikuai8.com>
---
 net/ipv4/netfilter/nf_nat_masquerade_ipv4.c | 2 ++
 net/ipv6/netfilter/nf_nat_masquerade_ipv6.c | 6 +++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/netfilter/nf_nat_masquerade_ipv4.c b/net/ipv4/netfilter/nf_nat_masquerade_ipv4.c
index ea91058..353ca0c 100644
--- a/net/ipv4/netfilter/nf_nat_masquerade_ipv4.c
+++ b/net/ipv4/netfilter/nf_nat_masquerade_ipv4.c
@@ -38,6 +38,8 @@
 
 	ct = nf_ct_get(skb, &ctinfo);
 	nat = nfct_nat(ct);
+	if (!nat)
+		return NF_ACCEPT;
 
 	NF_CT_ASSERT(ct && (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED ||
 			    ctinfo == IP_CT_RELATED_REPLY));
diff --git a/net/ipv6/netfilter/nf_nat_masquerade_ipv6.c b/net/ipv6/netfilter/nf_nat_masquerade_ipv6.c
index 051b6a6..875e776 100644
--- a/net/ipv6/netfilter/nf_nat_masquerade_ipv6.c
+++ b/net/ipv6/netfilter/nf_nat_masquerade_ipv6.c
@@ -32,17 +32,21 @@
 	enum ip_conntrack_info ctinfo;
 	struct in6_addr src;
 	struct nf_conn *ct;
+	struct nf_conn_nat *nat;
 	struct nf_nat_range newrange;
 
 	ct = nf_ct_get(skb, &ctinfo);
 	NF_CT_ASSERT(ct && (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED ||
 			    ctinfo == IP_CT_RELATED_REPLY));
+	nat = nfct_nat(ct);
+	if (!nat)
+		return NF_ACCEPT;
 
 	if (ipv6_dev_get_saddr(nf_ct_net(ct), out,
 			       &ipv6_hdr(skb)->daddr, 0, &src) < 0)
 		return NF_DROP;
 
-	nfct_nat(ct)->masq_index = out->ifindex;
+	nat->masq_index = out->ifindex;
 
 	newrange.flags		= range->flags | NF_NAT_RANGE_MAP_IPS;
 	newrange.min_addr.in6	= src;
-- 
1.9.1

Re: [PATCH nf 1/1] netfilter: nat_masquerade: Check oom when invoke nfct_nat

[Pablo Neira Ayuso]

On Fri, Mar 03, 2017 at 09:58:52AM +0800, fgao@ikuai8.com wrote:
> From: Gao Feng <fgao@ikuai8.com>
> 
> When memory is exhausted, nf_ct_nat_ext_add may return NULL. Then
> nf_nat_ipv4_fn and nf_nat_ipv6_fn would return NF_ACCEPT in this
> case.
> So we need add the NULL check when invoke nfct_nat in these two
> functions.
> 
> Signed-off-by: Gao Feng <fgao@ikuai8.com>
> ---
>  net/ipv4/netfilter/nf_nat_masquerade_ipv4.c | 2 ++
>  net/ipv6/netfilter/nf_nat_masquerade_ipv6.c | 6 +++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/netfilter/nf_nat_masquerade_ipv4.c b/net/ipv4/netfilter/nf_nat_masquerade_ipv4.c
> index ea91058..353ca0c 100644
> --- a/net/ipv4/netfilter/nf_nat_masquerade_ipv4.c
> +++ b/net/ipv4/netfilter/nf_nat_masquerade_ipv4.c
> @@ -38,6 +38,8 @@
>  
>  	ct = nf_ct_get(skb, &ctinfo);
>  	nat = nfct_nat(ct);
> +	if (!nat)
> +		return NF_ACCEPT;

The masquerade target is only called from the nat table. If you look
at nf_nat_ipv4_fn(), that is called whenever the packet enters a nat
table, you can indeed check that nfct_nat() must be already there by
when we call masquerade.

So this is entirely unnecessary.

Re: [PATCH nf 1/1] netfilter: nat_masquerade: Check oom when invoke nfct_nat

[Gao Feng]

Hi Pablo,

On Fri, Mar 3, 2017 at 5:30 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Fri, Mar 03, 2017 at 09:58:52AM +0800, fgao@ikuai8.com wrote:
>> From: Gao Feng <fgao@ikuai8.com>
>>
>> When memory is exhausted, nf_ct_nat_ext_add may return NULL. Then
>> nf_nat_ipv4_fn and nf_nat_ipv6_fn would return NF_ACCEPT in this
>> case.
>> So we need add the NULL check when invoke nfct_nat in these two
>> functions.
>>
>> Signed-off-by: Gao Feng <fgao@ikuai8.com>
>> ---
>>  net/ipv4/netfilter/nf_nat_masquerade_ipv4.c | 2 ++
>>  net/ipv6/netfilter/nf_nat_masquerade_ipv6.c | 6 +++++-
>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/ipv4/netfilter/nf_nat_masquerade_ipv4.c b/net/ipv4/netfilter/nf_nat_masquerade_ipv4.c
>> index ea91058..353ca0c 100644
>> --- a/net/ipv4/netfilter/nf_nat_masquerade_ipv4.c
>> +++ b/net/ipv4/netfilter/nf_nat_masquerade_ipv4.c
>> @@ -38,6 +38,8 @@
>>
>>       ct = nf_ct_get(skb, &ctinfo);
>>       nat = nfct_nat(ct);
>> +     if (!nat)
>> +             return NF_ACCEPT;
>
> The masquerade target is only called from the nat table. If you look
> at nf_nat_ipv4_fn(), that is called whenever the packet enters a nat
> table, you can indeed check that nfct_nat() must be already there by
> when we call masquerade.
>
> So this is entirely unnecessary.

Thanks your answer.
I should find the masquerade only is called nat table.
And the nf_nat_ipv4_fn function should make sure the nat is valid .

Thanks again.

Regards
Feng