[PATCH nf-next 1/1] netfilter: ctlink: Return error directly when create expect without help

[PATCH nf-next 1/1] netfilter: ctlink: Return error directly when create expect without help

[gfree.wind]

From: Gao Feng <fgao@ikuai8.com>

The expect check func "__nf_ct_expect_check" asks the master_help is
necessary. So it is unnecessary to go ahead in ctnetlink_alloc_expect
when there is no help.

Actually the commit bc01befdcf3e ("netfilter: ctnetlink: add support
for user-space expectation helpers") permits ctlink create one expect
even though there is no master help. But the latter commit 3d058d7bc2c5
("netfilter: rework user-space expectation helper support") disables
it again.

Signed-off-by: Gao Feng <fgao@ikuai8.com>
---
 net/netfilter/nf_conntrack_netlink.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index d49cc1e..c9af545 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -3049,18 +3049,8 @@ static int ctnetlink_del_expect(struct net *net, struct sock *ctnl,
 
 	help = nfct_help(ct);
 	if (!help) {
-		if (!cda[CTA_EXPECT_TIMEOUT]) {
-			err = -EINVAL;
-			goto err_out;
-		}
-		exp->timeout.expires =
-		  jiffies + ntohl(nla_get_be32(cda[CTA_EXPECT_TIMEOUT])) * HZ;
-
-		exp->flags = NF_CT_EXPECT_USERSPACE;
-		if (cda[CTA_EXPECT_FLAGS]) {
-			exp->flags |=
-				ntohl(nla_get_be32(cda[CTA_EXPECT_FLAGS]));
-		}
+		err = -EOPNOTSUPP;
+		goto err_out;
 	} else {
 		if (cda[CTA_EXPECT_FLAGS]) {
 			exp->flags = ntohl(nla_get_be32(cda[CTA_EXPECT_FLAGS]));
-- 
1.9.1

Re: [PATCH nf-next 1/1] netfilter: ctlink: Return error directly when create expect without help

[Pablo Neira Ayuso]

[-- Attachment #1: Type: text/plain, Size: 662 bytes --]

On Tue, Mar 28, 2017 at 09:52:52AM +0800, gfree.wind@foxmail.com wrote:
> From: Gao Feng <fgao@ikuai8.com>
> 
> The expect check func "__nf_ct_expect_check" asks the master_help is
> necessary. So it is unnecessary to go ahead in ctnetlink_alloc_expect
> when there is no help.
> 
> Actually the commit bc01befdcf3e ("netfilter: ctnetlink: add support
> for user-space expectation helpers") permits ctlink create one expect
> even though there is no master help. But the latter commit 3d058d7bc2c5
> ("netfilter: rework user-space expectation helper support") disables
> it again.

Probably reject this upfront if no nfct_help(ct) is there.

See patch attached.

[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 2285 bytes --]

commit 543b8a39eaacef70822f23f8e8de336c18761c40
Author: Gao Feng <fgao@ikuai8.com>
Date:   Tue Mar 28 09:52:52 2017 +0800

    netfilter: ctnetlink: Expectations must have a conntrack helper area
    
    The expect check function __nf_ct_expect_check() asks the master_help
    is necessary. So it is unnecessary to go ahead in ctnetlink_alloc_expect
    when there is no help.
    
    Actually the commit bc01befdcf3e ("netfilter: ctnetlink: add support
    for user-space expectation helpers") permits ctnetlink create one expect
    even though there is no master help. But the latter commit 3d058d7bc2c5
    ("netfilter: rework user-space expectation helper support") disables
    it again.
    
    Signed-off-by: Gao Feng <fgao@ikuai8.com>
    Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index ecdc324c7785..cd0a6d270ebe 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -3038,6 +3038,10 @@ ctnetlink_alloc_expect(const struct nlattr * const cda[], struct nf_conn *ct,
 	struct nf_conn_help *help;
 	int err;
 
+	help = nfct_help(ct);
+	if (!help)
+		return ERR_PTR(-EOPNOTSUPP);
+
 	if (cda[CTA_EXPECT_CLASS] && helper) {
 		class = ntohl(nla_get_be32(cda[CTA_EXPECT_CLASS]));
 		if (class > helper->expect_class_max)
@@ -3047,26 +3051,11 @@ ctnetlink_alloc_expect(const struct nlattr * const cda[], struct nf_conn *ct,
 	if (!exp)
 		return ERR_PTR(-ENOMEM);
 
-	help = nfct_help(ct);
-	if (!help) {
-		if (!cda[CTA_EXPECT_TIMEOUT]) {
-			err = -EINVAL;
-			goto err_out;
-		}
-		exp->timeout.expires =
-		  jiffies + ntohl(nla_get_be32(cda[CTA_EXPECT_TIMEOUT])) * HZ;
-
-		exp->flags = NF_CT_EXPECT_USERSPACE;
-		if (cda[CTA_EXPECT_FLAGS]) {
-			exp->flags |=
-				ntohl(nla_get_be32(cda[CTA_EXPECT_FLAGS]));
-		}
+	if (cda[CTA_EXPECT_FLAGS]) {
+		exp->flags = ntohl(nla_get_be32(cda[CTA_EXPECT_FLAGS]));
+		exp->flags &= ~NF_CT_EXPECT_USERSPACE;
 	} else {
-		if (cda[CTA_EXPECT_FLAGS]) {
-			exp->flags = ntohl(nla_get_be32(cda[CTA_EXPECT_FLAGS]));
-			exp->flags &= ~NF_CT_EXPECT_USERSPACE;
-		} else
-			exp->flags = 0;
+		exp->flags = 0;
 	}
 	if (cda[CTA_EXPECT_FN]) {
 		const char *name = nla_data(cda[CTA_EXPECT_FN]);

RE: [PATCH nf-next 1/1] netfilter: ctlink: Return error directly when create expect without help

[Gao Feng]

Hi Pablo,

> -----Original Message-----
> From: Pablo Neira Ayuso [mailto:pablo@netfilter.org]
> Sent: Friday, April 7, 2017 3:55 AM
> To: gfree.wind@foxmail.com
> Cc: netfilter-devel@vger.kernel.org; Gao Feng <fgao@ikuai8.com>
> Subject: Re: [PATCH nf-next 1/1] netfilter: ctlink: Return error directly
when
> create expect without help
> 
> On Tue, Mar 28, 2017 at 09:52:52AM +0800, gfree.wind@foxmail.com wrote:
> > From: Gao Feng <fgao@ikuai8.com>
> >
> > The expect check func "__nf_ct_expect_check" asks the master_help is
> > necessary. So it is unnecessary to go ahead in ctnetlink_alloc_expect
> > when there is no help.
> >
> > Actually the commit bc01befdcf3e ("netfilter: ctnetlink: add support
> > for user-space expectation helpers") permits ctlink create one expect
> > even though there is no master help. But the latter commit
> > 3d058d7bc2c5
> > ("netfilter: rework user-space expectation helper support") disables
> > it again.
> 
> Probably reject this upfront if no nfct_help(ct) is there.
> 
> See patch attached.
Yes, it is better.
Would you apply this patch which you attached directly, or need I send
another v2 update?

Best Regards
Feng

Re: [PATCH nf-next 1/1] netfilter: ctlink: Return error directly when create expect without help

[Pablo Neira Ayuso]

On Fri, Apr 07, 2017 at 06:19:07AM +0800, Gao Feng wrote:
> Yes, it is better.
> Would you apply this patch which you attached directly, or need I send
> another v2 update?

I'll apply this one. No need to send v2.