netdev - Re: [PATCHv3] netns: oops in ip[6]_frag

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090317132113.GA6939@ff.dom.local>
Date:	Tue, 17 Mar 2009 13:21:13 +0000
From:	Jarek Poplawski <jarkao2@...il.com>
To:	"Jorge Boncompte [DTI2]" <jorge@...2.net>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCHv3] netns: oops in ip[6]_frag_reasm incrementing stats

On Tue, Mar 17, 2009 at 12:55:42PM +0100, Jorge Boncompte [DTI2] wrote:
> dev can be NULL in ip[6]_frag_reasm for skb's coming from RAW sockets.
>
> Quagga's OSPFD sends fragmented packets on a RAW socket, when netfilter
> conntrack reassembles them on the OUTPUT path you hit this code path.
>
> You can test it with something like "hping2 -0 -d 2000 -f AA.BB.CC.DD"
>
> Changes from v2: (address comments from Jarek Poplawski)
> 	- Patch reworked to get the net pointer with container_of()
> 	  instead of passing it to function calls.
> 	- Fix IPv6 code
> Changes from v1:
> 	- Fixed description

I guess David will be interested only with the final state of changes,
so v1 & v2 are not necessary here...

Anyway, ipv4 looks OK to me, but ipv6 looks like something is
different:
> +	IP6_INC_STATS_BH(net, __in6_dev_get(dev), IPSTATS_MIB_REASMFAILS);

It still depends on dev != NULL in __in6_dev_get(). I see there
is also used skb->dst for similar things in ip6_frag_queue(), so I
don't know: it needs rethinking, and maybe these patches should be
separated if you prefer.

Thanks,
Jarek P.

>
> Signed-off-by: Jorge Boncompte [DTI2] <jorge@...2.net>
> ---
> net/ipv4/ip_fragment.c |    5 +++--
> net/ipv6/reassembly.c  |    7 +++----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
> index 6659ac0..4806e33 100644
> --- a/net/ipv4/ip_fragment.c
> +++ b/net/ipv4/ip_fragment.c
> @@ -463,6 +463,7 @@ err:
> static int ip_frag_reasm(struct ipq *qp, struct sk_buff *prev,
> 			 struct net_device *dev)
> {
> +	struct net *net = container_of(qp->q.net, struct net, ipv4.frags);
> 	struct iphdr *iph;
> 	struct sk_buff *fp, *head = qp->q.fragments;
> 	int len;
> @@ -548,7 +549,7 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *prev,
> 	iph = ip_hdr(head);
> 	iph->frag_off = 0;
> 	iph->tot_len = htons(len);
> -	IP_INC_STATS_BH(dev_net(dev), IPSTATS_MIB_REASMOKS);
> +	IP_INC_STATS_BH(net, IPSTATS_MIB_REASMOKS);
> 	qp->q.fragments = NULL;
> 	return 0;
>
> @@ -562,7 +563,7 @@ out_oversize:
> 		printk(KERN_INFO "Oversized IP packet from %pI4.\n",
> 			&qp->saddr);
> out_fail:
> -	IP_INC_STATS_BH(dev_net(dev), IPSTATS_MIB_REASMFAILS);
> +	IP_INC_STATS_BH(net, IPSTATS_MIB_REASMFAILS);
> 	return err;
> }
>
> diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> index 3c57511..e9ac7a1 100644
> --- a/net/ipv6/reassembly.c
> +++ b/net/ipv6/reassembly.c
> @@ -452,6 +452,7 @@ err:
> static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> 			  struct net_device *dev)
> {
> +	struct net *net = container_of(fq->q.net, struct net, ipv6.frags);
> 	struct sk_buff *fp, *head = fq->q.fragments;
> 	int    payload_len;
> 	unsigned int nhoff;
> @@ -551,8 +552,7 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> 					  head->csum);
>
> 	rcu_read_lock();
> -	IP6_INC_STATS_BH(dev_net(dev),
> -			 __in6_dev_get(dev), IPSTATS_MIB_REASMOKS);
> +	IP6_INC_STATS_BH(net, __in6_dev_get(dev), IPSTATS_MIB_REASMOKS);
> 	rcu_read_unlock();
> 	fq->q.fragments = NULL;
> 	return 1;
> @@ -566,8 +566,7 @@ out_oom:
> 		printk(KERN_DEBUG "ip6_frag_reasm: no memory for reassembly\n");
> out_fail:
> 	rcu_read_lock();
> -	IP6_INC_STATS_BH(dev_net(dev),
> -			 __in6_dev_get(dev), IPSTATS_MIB_REASMFAILS);
> +	IP6_INC_STATS_BH(net, __in6_dev_get(dev), IPSTATS_MIB_REASMFAILS);
> 	rcu_read_unlock();
> 	return -1;
> }
> -- 
> 1.5.6.5
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html