All of lore.kernel.org
 help / color / mirror / Atom feed
* Doubt about CTA_EXPECT_* values passed to ctnetlink_parse_tuple()
@ 2017-04-17 22:37 Matthias Kaehlcke
  2017-04-18  8:39 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 5+ messages in thread
From: Matthias Kaehlcke @ 2017-04-17 22:37 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Jozsef Kadlecsik; +Cc: netfilter-devel, coreteam

Hi,

While working on clang support for kernel builds I came across at what
at first sight looks like an enum mismatch in the netfilter conntrack
code:

In multiple occasions CTA_EXPECT_* values (of type enum ctattr_expect)
are passed to ctnetlink_parse_tuple(), which expects an 'enum
ctattr_type' as type argument.

In other cases unintentional use of a wrong enum type maybe be
obvious, in this case however it isn't clear to me which 'enum
ctattr_type' should be passed instead, so I wonder if the use of the
different type is intended.

Could someone please briefly clarify? My background on netfilter
internals is zero, so please forgive my probably naive question.

Thanks

Matthias

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: Doubt about CTA_EXPECT_* values passed to ctnetlink_parse_tuple()
  2017-04-17 22:37 Doubt about CTA_EXPECT_* values passed to ctnetlink_parse_tuple() Matthias Kaehlcke
@ 2017-04-18  8:39 ` Pablo Neira Ayuso
  2017-04-18 19:41   ` Matthias Kaehlcke
  0 siblings, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2017-04-18  8:39 UTC (permalink / raw)
  To: Matthias Kaehlcke; +Cc: Jozsef Kadlecsik, netfilter-devel, coreteam

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

Hi Mattias,

On Mon, Apr 17, 2017 at 03:37:30PM -0700, Matthias Kaehlcke wrote:
> Hi,
> 
> While working on clang support for kernel builds I came across at what
> at first sight looks like an enum mismatch in the netfilter conntrack
> code:
> 
> In multiple occasions CTA_EXPECT_* values (of type enum ctattr_expect)
> are passed to ctnetlink_parse_tuple(), which expects an 'enum
> ctattr_type' as type argument.

Are you refering to this? See patch.

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

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index dc7dfd68fafe..88d7cd9b6b0a 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1006,9 +1006,8 @@ static const struct nla_policy tuple_nla_policy[CTA_TUPLE_MAX+1] = {
 
 static int
 ctnetlink_parse_tuple(const struct nlattr * const cda[],
-		      struct nf_conntrack_tuple *tuple,
-		      enum ctattr_type type, u_int8_t l3num,
-		      struct nf_conntrack_zone *zone)
+		      struct nf_conntrack_tuple *tuple, u32 type,
+		      u_int8_t l3num, struct nf_conntrack_zone *zone)
 {
 	struct nlattr *tb[CTA_TUPLE_MAX+1];
 	int err;

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: Doubt about CTA_EXPECT_* values passed to ctnetlink_parse_tuple()
  2017-04-18  8:39 ` Pablo Neira Ayuso
@ 2017-04-18 19:41   ` Matthias Kaehlcke
  2017-04-18 19:43     ` Matthias Kaehlcke
  0 siblings, 1 reply; 5+ messages in thread
From: Matthias Kaehlcke @ 2017-04-18 19:41 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Jozsef Kadlecsik, netfilter-devel, coreteam, Matthias Kaehlcke

Hi Pablo,

Thanks for your reply!

El Tue, Apr 18, 2017 at 10:39:47AM +0200 Pablo Neira Ayuso ha dit:

> On Mon, Apr 17, 2017 at 03:37:30PM -0700, Matthias Kaehlcke wrote:
> > Hi,
> > 
> > While working on clang support for kernel builds I came across at what
> > at first sight looks like an enum mismatch in the netfilter conntrack
> > code:
> > 
> > In multiple occasions CTA_EXPECT_* values (of type enum ctattr_expect)
> > are passed to ctnetlink_parse_tuple(), which expects an 'enum
> > ctattr_type' as type argument.
> 
> Are you refering to this? See patch.

Almost, see patch :)

I interpret that passing the different enum types is intentional and
changing the parameter type an acceptable solution.

Cheers

Matthias

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: Doubt about CTA_EXPECT_* values passed to ctnetlink_parse_tuple()
  2017-04-18 19:41   ` Matthias Kaehlcke
@ 2017-04-18 19:43     ` Matthias Kaehlcke
  2017-04-19  8:58       ` Pablo Neira Ayuso
  0 siblings, 1 reply; 5+ messages in thread
From: Matthias Kaehlcke @ 2017-04-18 19:43 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Jozsef Kadlecsik, netfilter-devel, coreteam

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

El Tue, Apr 18, 2017 at 12:41:16PM -0700 Matthias Kaehlcke ha dit:

> Hi Pablo,
> 
> Thanks for your reply!
> 
> El Tue, Apr 18, 2017 at 10:39:47AM +0200 Pablo Neira Ayuso ha dit:
> 
> > On Mon, Apr 17, 2017 at 03:37:30PM -0700, Matthias Kaehlcke wrote:
> > > Hi,
> > > 
> > > While working on clang support for kernel builds I came across at what
> > > at first sight looks like an enum mismatch in the netfilter conntrack
> > > code:
> > > 
> > > In multiple occasions CTA_EXPECT_* values (of type enum ctattr_expect)
> > > are passed to ctnetlink_parse_tuple(), which expects an 'enum
> > > ctattr_type' as type argument.
> > 
> > Are you refering to this? See patch.
> 
> Almost, see patch :)

This time with patch

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

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 27540455dc62..4d7f3780d64b 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1006,9 +1006,8 @@ static const struct nla_policy tuple_nla_policy[CTA_TUPLE_MAX+1] = {
 
 static int
 ctnetlink_parse_tuple(const struct nlattr * const cda[],
-		      struct nf_conntrack_tuple *tuple,
-		      enum ctattr_type type, u_int8_t l3num,
-		      struct nf_conntrack_zone *zone)
+		      struct nf_conntrack_tuple *tuple, u32 type,
+		      u_int8_t l3num, struct nf_conntrack_zone *zone)
 {
 	struct nlattr *tb[CTA_TUPLE_MAX+1];
 	int err;
@@ -2405,7 +2404,7 @@ static struct nfnl_ct_hook ctnetlink_glue_hook = {
 
 static int ctnetlink_exp_dump_tuple(struct sk_buff *skb,
 				    const struct nf_conntrack_tuple *tuple,
-				    enum ctattr_expect type)
+				    uint32_t type)
 {
 	struct nlattr *nest_parms;
 

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: Doubt about CTA_EXPECT_* values passed to ctnetlink_parse_tuple()
  2017-04-18 19:43     ` Matthias Kaehlcke
@ 2017-04-19  8:58       ` Pablo Neira Ayuso
  0 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2017-04-19  8:58 UTC (permalink / raw)
  To: Matthias Kaehlcke; +Cc: Jozsef Kadlecsik, netfilter-devel, coreteam

On Tue, Apr 18, 2017 at 12:43:15PM -0700, Matthias Kaehlcke wrote:
> El Tue, Apr 18, 2017 at 12:41:16PM -0700 Matthias Kaehlcke ha dit:
> 
> > Hi Pablo,
> > 
> > Thanks for your reply!
> > 
> > El Tue, Apr 18, 2017 at 10:39:47AM +0200 Pablo Neira Ayuso ha dit:
> > 
> > > On Mon, Apr 17, 2017 at 03:37:30PM -0700, Matthias Kaehlcke wrote:
> > > > Hi,
> > > > 
> > > > While working on clang support for kernel builds I came across at what
> > > > at first sight looks like an enum mismatch in the netfilter conntrack
> > > > code:
> > > > 
> > > > In multiple occasions CTA_EXPECT_* values (of type enum ctattr_expect)
> > > > are passed to ctnetlink_parse_tuple(), which expects an 'enum
> > > > ctattr_type' as type argument.
> > > 
> > > Are you refering to this? See patch.
> > 
> > Almost, see patch :)
> 
> This time with patch

> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> index 27540455dc62..4d7f3780d64b 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -1006,9 +1006,8 @@ static const struct nla_policy tuple_nla_policy[CTA_TUPLE_MAX+1] = {
>  
>  static int
>  ctnetlink_parse_tuple(const struct nlattr * const cda[],
> -		      struct nf_conntrack_tuple *tuple,
> -		      enum ctattr_type type, u_int8_t l3num,
> -		      struct nf_conntrack_zone *zone)
> +		      struct nf_conntrack_tuple *tuple, u32 type,
> +		      u_int8_t l3num, struct nf_conntrack_zone *zone)
>  {
>  	struct nlattr *tb[CTA_TUPLE_MAX+1];
>  	int err;
> @@ -2405,7 +2404,7 @@ static struct nfnl_ct_hook ctnetlink_glue_hook = {
>  
>  static int ctnetlink_exp_dump_tuple(struct sk_buff *skb,
>  				    const struct nf_conntrack_tuple *tuple,
> -				    enum ctattr_expect type)
> +				    uint32_t type)
>  {
>  	struct nlattr *nest_parms;
>  

Looks fine. Please, add title, description and Signed-off-by tag, I'll
take it :)

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-04-19  8:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-17 22:37 Doubt about CTA_EXPECT_* values passed to ctnetlink_parse_tuple() Matthias Kaehlcke
2017-04-18  8:39 ` Pablo Neira Ayuso
2017-04-18 19:41   ` Matthias Kaehlcke
2017-04-18 19:43     ` Matthias Kaehlcke
2017-04-19  8:58       ` 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.