All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] netfilter: conntrack: Fix ifdef checks for CONFIG_NF_CONNTRACK_MARK
@ 2016-12-14 21:35 joseph.j.conley
  2016-12-15 20:55 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 4+ messages in thread
From: joseph.j.conley @ 2016-12-14 21:35 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Joe Conley

From: Joe Conley <joe.conley@lairdtech.com>

Two missing ifdef checks for CONFIG_NF_CONNTRACK_MARK were causing
EOPNOTSUPP to be returned. Every single place that cda[CTA_MARK] or cda[CTA_MARK_MASK]
was checked was inside a #ifdef for CONFIG_NF_CONNTRACK_MARK except for these
two places. The reason for this change stems from this commit:
866476f323465a8afef10b14b48d5136bf5c51fe (netfilter: conntrack: Flush connections with a given mark)

This allows conntrack -L to be ran succesfully when CONFIG_NF_CONNTRACK_MARK
is not enabled.

Signed-off-by: Joe Conley <joe.conley@lairdtech.com>
---
 net/netfilter/nf_conntrack_netlink.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 2754045..94146bd 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1107,13 +1107,13 @@ static int ctnetlink_flush_conntrack(struct net *net,
 				     u32 portid, int report)
 {
 	struct ctnetlink_filter *filter = NULL;
-
+#ifdef CONFIG_NF_CONNTRACK_MARK
 	if (cda[CTA_MARK] && cda[CTA_MARK_MASK]) {
 		filter = ctnetlink_alloc_filter(cda);
 		if (IS_ERR(filter))
 			return PTR_ERR(filter);
 	}
-
+#endif
 	nf_ct_iterate_cleanup(net, ctnetlink_filter_match, filter,
 			      portid, report);
 	kfree(filter);
@@ -1192,7 +1192,7 @@ static int ctnetlink_get_conntrack(struct net *net, struct sock *ctnl,
 			.dump = ctnetlink_dump_table,
 			.done = ctnetlink_done,
 		};
-
+#ifdef CONFIG_NF_CONNTRACK_MARK
 		if (cda[CTA_MARK] && cda[CTA_MARK_MASK]) {
 			struct ctnetlink_filter *filter;

@@ -1202,6 +1202,7 @@ static int ctnetlink_get_conntrack(struct net *net, struct sock *ctnl,

 			c.data = filter;
 		}
+#endif
 		return netlink_dump_start(ctnl, skb, nlh, &c);
 	}

--
2.7.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] netfilter: conntrack: Fix ifdef checks for CONFIG_NF_CONNTRACK_MARK
  2016-12-14 21:35 [PATCH] netfilter: conntrack: Fix ifdef checks for CONFIG_NF_CONNTRACK_MARK joseph.j.conley
@ 2016-12-15 20:55 ` Pablo Neira Ayuso
  2016-12-19 15:40   ` Joseph Conley
  0 siblings, 1 reply; 4+ messages in thread
From: Pablo Neira Ayuso @ 2016-12-15 20:55 UTC (permalink / raw)
  To: joseph.j.conley; +Cc: netfilter-devel, Joe Conley

On Wed, Dec 14, 2016 at 04:35:57PM -0500, joseph.j.conley@gmail.com wrote:
> From: Joe Conley <joe.conley@lairdtech.com>
> 
> Two missing ifdef checks for CONFIG_NF_CONNTRACK_MARK were causing
> EOPNOTSUPP to be returned. Every single place that cda[CTA_MARK] or cda[CTA_MARK_MASK]
> was checked was inside a #ifdef for CONFIG_NF_CONNTRACK_MARK except for these
> two places. The reason for this change stems from this commit:
> 866476f323465a8afef10b14b48d5136bf5c51fe (netfilter: conntrack: Flush connections with a given mark)
> 
> This allows conntrack -L to be ran succesfully when CONFIG_NF_CONNTRACK_MARK
> is not enabled.

I would like to understand how you're triggering this problem. If it
is a plain 'conntrack -L' command line invocation that triggers the
problem, then it's probably a userspace problem since we should not
send any mark attribute to the kernel if not set.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] netfilter: conntrack: Fix ifdef checks for CONFIG_NF_CONNTRACK_MARK
  2016-12-15 20:55 ` Pablo Neira Ayuso
@ 2016-12-19 15:40   ` Joseph Conley
  2016-12-23 14:55     ` Pablo Neira Ayuso
  0 siblings, 1 reply; 4+ messages in thread
From: Joseph Conley @ 2016-12-19 15:40 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel, Joe Conley

On Thu, Dec 15, 2016 at 3:55 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> On Wed, Dec 14, 2016 at 04:35:57PM -0500, joseph.j.conley@gmail.com wrote:
> > From: Joe Conley <joe.conley@lairdtech.com>
> >
> > Two missing ifdef checks for CONFIG_NF_CONNTRACK_MARK were causing
> > EOPNOTSUPP to be returned. Every single place that cda[CTA_MARK] or cda[CTA_MARK_MASK]
> > was checked was inside a #ifdef for CONFIG_NF_CONNTRACK_MARK except for these
> > two places. The reason for this change stems from this commit:
> > 866476f323465a8afef10b14b48d5136bf5c51fe (netfilter: conntrack: Flush connections with a given mark)
> >
> > This allows conntrack -L to be ran succesfully when CONFIG_NF_CONNTRACK_MARK
> > is not enabled.
>
> I would like to understand how you're triggering this problem. If it
> is a plain 'conntrack -L' command line invocation that triggers the
> problem, then it's probably a userspace problem since we should not
> send any mark attribute to the kernel if not set.

Building the kernel with CONFIG_NF_CONNTRACK_MARK disabled will cause
conntrack -L to return EOPNOTSUPP because of the missing ifdef checks.
Building the kernel with it enabled allows conntrack -L to run
successfully. At first, I thought this was a userspace bug as well but
it is not. Every single place CTA_MARK or CTA_MARK_MASK is used is
inside an ifdef check for CONFIG_NF_CONNTRACK_MARK except for these
two places. There is no clear reason as to why. There is no reason
that conntrack -L should return EOPNOTSUPP when
CONFIG_NF_CONNTRACK_MARK is disabled.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] netfilter: conntrack: Fix ifdef checks for CONFIG_NF_CONNTRACK_MARK
  2016-12-19 15:40   ` Joseph Conley
@ 2016-12-23 14:55     ` Pablo Neira Ayuso
  0 siblings, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2016-12-23 14:55 UTC (permalink / raw)
  To: Joseph Conley; +Cc: netfilter-devel, Joe Conley

On Mon, Dec 19, 2016 at 10:40:50AM -0500, Joseph Conley wrote:
> On Thu, Dec 15, 2016 at 3:55 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >
> > On Wed, Dec 14, 2016 at 04:35:57PM -0500, joseph.j.conley@gmail.com wrote:
> > > From: Joe Conley <joe.conley@lairdtech.com>
> > >
> > > Two missing ifdef checks for CONFIG_NF_CONNTRACK_MARK were causing
> > > EOPNOTSUPP to be returned. Every single place that cda[CTA_MARK] or cda[CTA_MARK_MASK]
> > > was checked was inside a #ifdef for CONFIG_NF_CONNTRACK_MARK except for these
> > > two places. The reason for this change stems from this commit:
> > > 866476f323465a8afef10b14b48d5136bf5c51fe (netfilter: conntrack: Flush connections with a given mark)
> > >
> > > This allows conntrack -L to be ran succesfully when CONFIG_NF_CONNTRACK_MARK
> > > is not enabled.
> >
> > I would like to understand how you're triggering this problem. If it
> > is a plain 'conntrack -L' command line invocation that triggers the
> > problem, then it's probably a userspace problem since we should not
> > send any mark attribute to the kernel if not set.
> 
> Building the kernel with CONFIG_NF_CONNTRACK_MARK disabled will cause
> conntrack -L to return EOPNOTSUPP because of the missing ifdef checks.
> Building the kernel with it enabled allows conntrack -L to run
> successfully. At first, I thought this was a userspace bug as well but
> it is not. Every single place CTA_MARK or CTA_MARK_MASK is used is
> inside an ifdef check for CONFIG_NF_CONNTRACK_MARK except for these
> two places. There is no clear reason as to why. There is no reason
> that conntrack -L should return EOPNOTSUPP when
> CONFIG_NF_CONNTRACK_MARK is disabled.

I think this a userspace bug, since we shouldn't send mark filter if
not set by the user.

Kernel rejects with EOPNOTSUPP to indicate userspace that the mark
filtering that requests is not supported. This check is good so user
knows that this mark-based filtering doesn't work.

Just sent a patch for userspace, thanks a lot for reporting.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-12-23 14:55 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-14 21:35 [PATCH] netfilter: conntrack: Fix ifdef checks for CONFIG_NF_CONNTRACK_MARK joseph.j.conley
2016-12-15 20:55 ` Pablo Neira Ayuso
2016-12-19 15:40   ` Joseph Conley
2016-12-23 14:55     ` Pablo Neira Ayuso

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.