netdev - Re: [PATCH nf-next 0/4] netfilter: rework netfilter ipv6 defrag

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: <20151020205306.GK4386@breakpoint.cc>
Date:	Tue, 20 Oct 2015 22:53:07 +0200
From:	Florian Westphal <fw@...len.de>
To:	Joe Stringer <joestringer@...ira.com>
Cc:	Florian Westphal <fw@...len.de>, netfilter-devel@...r.kernel.org,
	Linux Netdev List <netdev@...r.kernel.org>,
	Andy Zhou <azhou@...ira.com>
Subject: Re: [PATCH nf-next 0/4] netfilter: rework netfilter ipv6 defrag

Joe Stringer <joestringer@...ira.com> wrote:
> > Good point.  No, I don't.  Any suggestions?
> > I can try to just re-target -nf tree (sans patch #2).  Pablo?
> 
> The smallest change seems to be adding the nf_ct_frag6_consume_orig()
> call to OVS, plus the morph logic from patch 3. Alternatively if Pablo
> is fine with having the series re-targeted, then that sounds
> reasonable to me too.

Pablo, your call.

I would suggest to re-target patches #1 and #3 to nf tree, I can do
this, just let me know.

Alternative is to just add the nf_ct_frag6_consume_orig call to
openvswitch and handle that via net tree.

I can then wait for that change to pop up in nf-next and just resend
this series (which will then undo that change).

Let me know, thanks!

> > ipv4 side seems broken as well (ip_defrag frees skb on errors other than
> > -EINPROGRESS, so it looks like we will double-free in
> > do_execute_actions)
> 
> Oh dear. Thanks for the report. I propose wrapping the ip_defrag()
> with an skb_get()/skb_consume() as this seems to require the least
> invasive changes:
> 
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index a5ec34f8502f..0d2d24c99fd5 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -303,7 +303,11 @@ static int handle_fragments(struct net *net,
> struct sw_flow_key *key,
>                 int err;
> 
>                 memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));
> +
> +               skb_get(skb);
>                 err = ip_defrag(skb, user);
> +               if (!err || err == -EINPROGRESS)
> +                       consume_skb(skb);
>                 if (err)
>                         return err;

Indeed, that seems like the least invasive change.

Feel free to submit this to -net, there is no dependency on any of the
other changes.
--
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