[PATCH] netfilter: ctnetlink: Resolve conntrack L3-protocol flush regression

[PATCH] netfilter: ctnetlink: Resolve conntrack L3-protocol flush regression

[Kristian Evensen]

Commit 59c08c69c278 ("netfilter: ctnetlink: Support L3 protocol-filter
on flush") introduced a user-space regression when flushing connection
track entries. Before this commit, the nfgen_family field was not used
by the kernel and all entries were removed. Since this commit,
nfgen_family is used to filter out entries that should not be removed.
One example a broken tool is conntrack. conntrack always sets
nfgen_family to AF_INET, so after 59c08c69c278 only IPv4 entries were
removed with the -F parameter.

Pablo Neira Ayuso suggested using nfgenmsg->version to resolve the
regression, and this commit implements his suggestion. nfgenmsg->version
is so far set to zero, so it is well-suited to be used as a flag for
selecting old or new flush behavior. If version is 0, nfgen_family is
ignored and all entries are used. If user-space sets the version to one
(or any other value than 0), then the new behavior is used. As version
only can have two valid values, I chose not to add a new
NFNETLINK_VERSION-constant.

Fixes: 59c08c69c278 ("netfilter: ctnetlink: Support L3 protocol-filter
on flush")

Reported-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>
---
 net/netfilter/nf_conntrack_netlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 8dcc064d518d..7db79c1b8084 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1256,7 +1256,7 @@ static int ctnetlink_del_conntrack(struct net *net, struct sock *ctnl,
 	struct nf_conntrack_tuple tuple;
 	struct nf_conn *ct;
 	struct nfgenmsg *nfmsg = nlmsg_data(nlh);
-	u_int8_t u3 = nfmsg->nfgen_family;
+	u_int8_t u3 = nfmsg->version ? nfmsg->nfgen_family : AF_UNSPEC;
 	struct nf_conntrack_zone zone;
 	int err;
 
-- 
2.19.1

Re: [PATCH] netfilter: ctnetlink: Resolve conntrack L3-protocol flush regression

[Nicolas Dichtel]

Please, keep in CC all involved people.

Le 03/05/2019 à 17:40, Kristian Evensen a écrit :
> Commit 59c08c69c278 ("netfilter: ctnetlink: Support L3 protocol-filter
> on flush") introduced a user-space regression when flushing connection
> track entries. Before this commit, the nfgen_family field was not used
> by the kernel and all entries were removed. Since this commit,
> nfgen_family is used to filter out entries that should not be removed.
> One example a broken tool is conntrack. conntrack always sets
> nfgen_family to AF_INET, so after 59c08c69c278 only IPv4 entries were
> removed with the -F parameter.
> 
> Pablo Neira Ayuso suggested using nfgenmsg->version to resolve the
> regression, and this commit implements his suggestion. nfgenmsg->version
> is so far set to zero, so it is well-suited to be used as a flag for
> selecting old or new flush behavior. If version is 0, nfgen_family is
> ignored and all entries are used. If user-space sets the version to one
> (or any other value than 0), then the new behavior is used. As version
> only can have two valid values, I chose not to add a new
> NFNETLINK_VERSION-constant.
> 
> Fixes: 59c08c69c278 ("netfilter: ctnetlink: Support L3 protocol-filter
> on flush")
> 
Please, don't break the fixes line and don't separate it from other tags with an
empty line.

> Reported-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>
Tested-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Re: [PATCH] netfilter: ctnetlink: Resolve conntrack L3-protocol flush regression

[Pablo Neira Ayuso]

On Fri, May 03, 2019 at 07:02:54PM +0200, Nicolas Dichtel wrote:
> Please, keep in CC all involved people.
> 
> Le 03/05/2019 à 17:40, Kristian Evensen a écrit :
> > Commit 59c08c69c278 ("netfilter: ctnetlink: Support L3 protocol-filter
> > on flush") introduced a user-space regression when flushing connection
> > track entries. Before this commit, the nfgen_family field was not used
> > by the kernel and all entries were removed. Since this commit,
> > nfgen_family is used to filter out entries that should not be removed.
> > One example a broken tool is conntrack. conntrack always sets
> > nfgen_family to AF_INET, so after 59c08c69c278 only IPv4 entries were
> > removed with the -F parameter.
> > 
> > Pablo Neira Ayuso suggested using nfgenmsg->version to resolve the
> > regression, and this commit implements his suggestion. nfgenmsg->version
> > is so far set to zero, so it is well-suited to be used as a flag for
> > selecting old or new flush behavior. If version is 0, nfgen_family is
> > ignored and all entries are used. If user-space sets the version to one
> > (or any other value than 0), then the new behavior is used. As version
> > only can have two valid values, I chose not to add a new
> > NFNETLINK_VERSION-constant.
> > 
> > Fixes: 59c08c69c278 ("netfilter: ctnetlink: Support L3 protocol-filter
> > on flush")
> > 
> Please, don't break the fixes line and don't separate it from other tags with an
> empty line.

Will fix this before applying, no worries.

> > Reported-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> > Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>
> Tested-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Re: [PATCH] netfilter: ctnetlink: Resolve conntrack L3-protocol flush regression

[Kristian Evensen]

On Fri, May 3, 2019 at 7:05 PM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Will fix this before applying, no worries.

Thanks for taking care of my mistake :)

BR,
Kristian

Re: [PATCH] netfilter: ctnetlink: Resolve conntrack L3-protocol flush regression

[Pablo Neira Ayuso]

On Fri, May 03, 2019 at 05:40:07PM +0200, Kristian Evensen wrote:
> Commit 59c08c69c278 ("netfilter: ctnetlink: Support L3 protocol-filter
> on flush") introduced a user-space regression when flushing connection
> track entries. Before this commit, the nfgen_family field was not used
> by the kernel and all entries were removed. Since this commit,
> nfgen_family is used to filter out entries that should not be removed.
> One example a broken tool is conntrack. conntrack always sets
> nfgen_family to AF_INET, so after 59c08c69c278 only IPv4 entries were
> removed with the -F parameter.
> 
> Pablo Neira Ayuso suggested using nfgenmsg->version to resolve the
> regression, and this commit implements his suggestion. nfgenmsg->version
> is so far set to zero, so it is well-suited to be used as a flag for
> selecting old or new flush behavior. If version is 0, nfgen_family is
> ignored and all entries are used. If user-space sets the version to one
> (or any other value than 0), then the new behavior is used. As version
> only can have two valid values, I chose not to add a new
> NFNETLINK_VERSION-constant.

Applied, thanks.

Re: [PATCH] netfilter: ctnetlink: Resolve conntrack L3-protocol flush regression

[Nicolas Dichtel]

Le 06/05/2019 à 00:32, Pablo Neira Ayuso a écrit :
> On Fri, May 03, 2019 at 05:40:07PM +0200, Kristian Evensen wrote:
>> Commit 59c08c69c278 ("netfilter: ctnetlink: Support L3 protocol-filter
>> on flush") introduced a user-space regression when flushing connection
>> track entries. Before this commit, the nfgen_family field was not used
>> by the kernel and all entries were removed. Since this commit,
>> nfgen_family is used to filter out entries that should not be removed.
>> One example a broken tool is conntrack. conntrack always sets
>> nfgen_family to AF_INET, so after 59c08c69c278 only IPv4 entries were
>> removed with the -F parameter.
>>
>> Pablo Neira Ayuso suggested using nfgenmsg->version to resolve the
>> regression, and this commit implements his suggestion. nfgenmsg->version
>> is so far set to zero, so it is well-suited to be used as a flag for
>> selecting old or new flush behavior. If version is 0, nfgen_family is
>> ignored and all entries are used. If user-space sets the version to one
>> (or any other value than 0), then the new behavior is used. As version
>> only can have two valid values, I chose not to add a new
>> NFNETLINK_VERSION-constant.
> 
> Applied, thanks.
> 
Thank you.
Is it possible to queue this for stable?


Regards,
Nicolas

Re: [PATCH] netfilter: ctnetlink: Resolve conntrack L3-protocol flush regression

[Pablo Neira Ayuso]

On Mon, May 06, 2019 at 10:49:52AM +0200, Nicolas Dichtel wrote:
> Le 06/05/2019 à 00:32, Pablo Neira Ayuso a écrit :
> > On Fri, May 03, 2019 at 05:40:07PM +0200, Kristian Evensen wrote:
> >> Commit 59c08c69c278 ("netfilter: ctnetlink: Support L3 protocol-filter
> >> on flush") introduced a user-space regression when flushing connection
> >> track entries. Before this commit, the nfgen_family field was not used
> >> by the kernel and all entries were removed. Since this commit,
> >> nfgen_family is used to filter out entries that should not be removed.
> >> One example a broken tool is conntrack. conntrack always sets
> >> nfgen_family to AF_INET, so after 59c08c69c278 only IPv4 entries were
> >> removed with the -F parameter.
> >>
> >> Pablo Neira Ayuso suggested using nfgenmsg->version to resolve the
> >> regression, and this commit implements his suggestion. nfgenmsg->version
> >> is so far set to zero, so it is well-suited to be used as a flag for
> >> selecting old or new flush behavior. If version is 0, nfgen_family is
> >> ignored and all entries are used. If user-space sets the version to one
> >> (or any other value than 0), then the new behavior is used. As version
> >> only can have two valid values, I chose not to add a new
> >> NFNETLINK_VERSION-constant.
> > 
> > Applied, thanks.
> > 
> Thank you.
> Is it possible to queue this for stable?

Sure, as soon as this hits Linus' tree.

Re: [PATCH] netfilter: ctnetlink: Resolve conntrack L3-protocol flush regression

[Nicolas Dichtel]

Le 06/05/2019 à 15:16, Pablo Neira Ayuso a écrit :
> On Mon, May 06, 2019 at 10:49:52AM +0200, Nicolas Dichtel wrote:
[snip]
>> Is it possible to queue this for stable?
> 
> Sure, as soon as this hits Linus' tree.
> 
FYI, it's now in Linus tree:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f8e608982022


Thank you,
Nicolas

Re: [PATCH] netfilter: ctnetlink: Resolve conntrack L3-protocol flush regression

[Pablo Neira Ayuso]

On Mon, May 20, 2019 at 10:35:07AM +0200, Nicolas Dichtel wrote:
> Le 06/05/2019 à 15:16, Pablo Neira Ayuso a écrit :
> > On Mon, May 06, 2019 at 10:49:52AM +0200, Nicolas Dichtel wrote:
> [snip]
> >> Is it possible to queue this for stable?
> > 
> > Sure, as soon as this hits Linus' tree.
> > 
> FYI, it's now in Linus tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f8e608982022

Please, send an email requesting this to stable@vger.kernel.org and
keep me on CC.

I'll ACK it.

Thanks.

Re: [PATCH] netfilter: ctnetlink: Resolve conntrack L3-protocol flush regression

[Nicolas Dichtel]

Le 24/05/2019 à 11:22, Pablo Neira Ayuso a écrit :
> On Mon, May 20, 2019 at 10:35:07AM +0200, Nicolas Dichtel wrote:
>> Le 06/05/2019 à 15:16, Pablo Neira Ayuso a écrit :
>>> On Mon, May 06, 2019 at 10:49:52AM +0200, Nicolas Dichtel wrote:
>> [snip]
>>>> Is it possible to queue this for stable?
>>>
>>> Sure, as soon as this hits Linus' tree.
>>>
>> FYI, it's now in Linus tree:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f8e608982022
> 
> Please, send an email requesting this to stable@vger.kernel.org and
> keep me on CC.
This is a request to backport the upstream commit f8e608982022 ("netfilter:
ctnetlink: Resolve conntrack L3-protocol flush regression") in stable trees.


Thank you,
Nicolas

> 
> I'll ACK it.
> 
> Thanks.
> 

Re: [PATCH] netfilter: ctnetlink: Resolve conntrack L3-protocol flush regression

[Greg KH]

On Tue, May 28, 2019 at 03:57:37PM +0200, Nicolas Dichtel wrote:
> Le 24/05/2019 à 11:22, Pablo Neira Ayuso a écrit :
> > On Mon, May 20, 2019 at 10:35:07AM +0200, Nicolas Dichtel wrote:
> >> Le 06/05/2019 à 15:16, Pablo Neira Ayuso a écrit :
> >>> On Mon, May 06, 2019 at 10:49:52AM +0200, Nicolas Dichtel wrote:
> >> [snip]
> >>>> Is it possible to queue this for stable?
> >>>
> >>> Sure, as soon as this hits Linus' tree.
> >>>
> >> FYI, it's now in Linus tree:
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f8e608982022
> > 
> > Please, send an email requesting this to stable@vger.kernel.org and
> > keep me on CC.
> This is a request to backport the upstream commit f8e608982022 ("netfilter:
> ctnetlink: Resolve conntrack L3-protocol flush regression") in stable trees.

Now queued up, thanks.

greg k-h