netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* iptables userspace API broken due to added value in nf_inet_hooks
@ 2020-10-14 12:59 Jason A. Donenfeld
  2020-10-14 13:01 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 3+ messages in thread
From: Jason A. Donenfeld @ 2020-10-14 12:59 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel, Netdev

Hey Pablo,

In 60a3815da702fd9e4759945f26cce5c47d3967ad, you added another enum
value to nf_inet_hooks:

--- a/include/uapi/linux/netfilter.h
+++ b/include/uapi/linux/netfilter.h
@@ -45,6 +45,7 @@ enum nf_inet_hooks {
       NF_INET_FORWARD,
       NF_INET_LOCAL_OUT,
       NF_INET_POST_ROUTING,
+       NF_INET_INGRESS,
       NF_INET_NUMHOOKS
};

That seems fine, but actually it changes the value of
NF_INET_NUMHOOKS, which is used in struct ipt_getinfo:

/* The argument to IPT_SO_GET_INFO */
struct ipt_getinfo {
       /* Which table: caller fills this in. */
       char name[XT_TABLE_MAXNAMELEN];

       /* Kernel fills these in. */
       /* Which hook entry points are valid: bitmask */
       unsigned int valid_hooks;

       /* Hook entry points: one per netfilter hook. */
       unsigned int hook_entry[NF_INET_NUMHOOKS];

       /* Underflow points. */
       unsigned int underflow[NF_INET_NUMHOOKS];

       /* Number of entries */
       unsigned int num_entries;

       /* Size of entries. */
       unsigned int size;
};

This in turn makes that struct bigger, which means this check in
net/ipv4/netfilter/ip_tables.c fails:

static int get_info(struct net *net, void __user *user, const int *len)
{
       char name[XT_TABLE_MAXNAMELEN];
       struct xt_table *t;
       int ret;

       if (*len != sizeof(struct ipt_getinfo))
               return -EINVAL;

This is affecting my CI, which attempts to use an older iptables with
net-next and fails with:

iptables v1.8.4 (legacy): can't initialize iptables table `filter':
Module is wrong version
Perhaps iptables or your kernel needs to be upgraded.

Is this kind of breakage okay? If there's an exception carved out for
breaking the iptables API, just let me know, and I'll look into making
adjustments to work around it in my CI. On the other hand, if this
breakage was unintentional, now you know.

Jason

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

* Re: iptables userspace API broken due to added value in nf_inet_hooks
  2020-10-14 12:59 iptables userspace API broken due to added value in nf_inet_hooks Jason A. Donenfeld
@ 2020-10-14 13:01 ` Pablo Neira Ayuso
  2020-10-14 13:01   ` Pablo Neira Ayuso
  0 siblings, 1 reply; 3+ messages in thread
From: Pablo Neira Ayuso @ 2020-10-14 13:01 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: netfilter-devel, Netdev

On Wed, Oct 14, 2020 at 02:59:47PM +0200, Jason A. Donenfeld wrote:
> Hey Pablo,
> 
> In 60a3815da702fd9e4759945f26cce5c47d3967ad, you added another enum
> value to nf_inet_hooks:
> 
> --- a/include/uapi/linux/netfilter.h
> +++ b/include/uapi/linux/netfilter.h
> @@ -45,6 +45,7 @@ enum nf_inet_hooks {
>        NF_INET_FORWARD,
>        NF_INET_LOCAL_OUT,
>        NF_INET_POST_ROUTING,
> +       NF_INET_INGRESS,
>        NF_INET_NUMHOOKS
> };
> 
> That seems fine, but actually it changes the value of
> NF_INET_NUMHOOKS, which is used in struct ipt_getinfo:
> 
> /* The argument to IPT_SO_GET_INFO */
> struct ipt_getinfo {
>        /* Which table: caller fills this in. */
>        char name[XT_TABLE_MAXNAMELEN];
> 
>        /* Kernel fills these in. */
>        /* Which hook entry points are valid: bitmask */
>        unsigned int valid_hooks;
> 
>        /* Hook entry points: one per netfilter hook. */
>        unsigned int hook_entry[NF_INET_NUMHOOKS];
> 
>        /* Underflow points. */
>        unsigned int underflow[NF_INET_NUMHOOKS];
> 
>        /* Number of entries */
>        unsigned int num_entries;
> 
>        /* Size of entries. */
>        unsigned int size;
> };
> 
> This in turn makes that struct bigger, which means this check in
> net/ipv4/netfilter/ip_tables.c fails:
> 
> static int get_info(struct net *net, void __user *user, const int *len)
> {
>        char name[XT_TABLE_MAXNAMELEN];
>        struct xt_table *t;
>        int ret;
> 
>        if (*len != sizeof(struct ipt_getinfo))
>                return -EINVAL;
> 
> This is affecting my CI, which attempts to use an older iptables with
> net-next and fails with:
> 
> iptables v1.8.4 (legacy): can't initialize iptables table `filter':
> Module is wrong version
> Perhaps iptables or your kernel needs to be upgraded.
> 
> Is this kind of breakage okay? If there's an exception carved out for
> breaking the iptables API, just let me know, and I'll look into making
> adjustments to work around it in my CI. On the other hand, if this
> breakage was unintentional, now you know.

Oh right, I'll need a new IPT_INET_NUMHOOKS for this.

I'll submit a patch, that's for the heads up.

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

* Re: iptables userspace API broken due to added value in nf_inet_hooks
  2020-10-14 13:01 ` Pablo Neira Ayuso
@ 2020-10-14 13:01   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 3+ messages in thread
From: Pablo Neira Ayuso @ 2020-10-14 13:01 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: netfilter-devel, Netdev

On Wed, Oct 14, 2020 at 03:01:15PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Oct 14, 2020 at 02:59:47PM +0200, Jason A. Donenfeld wrote:
> > Hey Pablo,
> > 
> > In 60a3815da702fd9e4759945f26cce5c47d3967ad, you added another enum
> > value to nf_inet_hooks:
> > 
> > --- a/include/uapi/linux/netfilter.h
> > +++ b/include/uapi/linux/netfilter.h
> > @@ -45,6 +45,7 @@ enum nf_inet_hooks {
> >        NF_INET_FORWARD,
> >        NF_INET_LOCAL_OUT,
> >        NF_INET_POST_ROUTING,
> > +       NF_INET_INGRESS,
> >        NF_INET_NUMHOOKS
> > };
> > 
> > That seems fine, but actually it changes the value of
> > NF_INET_NUMHOOKS, which is used in struct ipt_getinfo:
> > 
> > /* The argument to IPT_SO_GET_INFO */
> > struct ipt_getinfo {
> >        /* Which table: caller fills this in. */
> >        char name[XT_TABLE_MAXNAMELEN];
> > 
> >        /* Kernel fills these in. */
> >        /* Which hook entry points are valid: bitmask */
> >        unsigned int valid_hooks;
> > 
> >        /* Hook entry points: one per netfilter hook. */
> >        unsigned int hook_entry[NF_INET_NUMHOOKS];
> > 
> >        /* Underflow points. */
> >        unsigned int underflow[NF_INET_NUMHOOKS];
> > 
> >        /* Number of entries */
> >        unsigned int num_entries;
> > 
> >        /* Size of entries. */
> >        unsigned int size;
> > };
> > 
> > This in turn makes that struct bigger, which means this check in
> > net/ipv4/netfilter/ip_tables.c fails:
> > 
> > static int get_info(struct net *net, void __user *user, const int *len)
> > {
> >        char name[XT_TABLE_MAXNAMELEN];
> >        struct xt_table *t;
> >        int ret;
> > 
> >        if (*len != sizeof(struct ipt_getinfo))
> >                return -EINVAL;
> > 
> > This is affecting my CI, which attempts to use an older iptables with
> > net-next and fails with:
> > 
> > iptables v1.8.4 (legacy): can't initialize iptables table `filter':
> > Module is wrong version
> > Perhaps iptables or your kernel needs to be upgraded.
> > 
> > Is this kind of breakage okay? If there's an exception carved out for
> > breaking the iptables API, just let me know, and I'll look into making
> > adjustments to work around it in my CI. On the other hand, if this
> > breakage was unintentional, now you know.
> 
> Oh right, I'll need a new IPT_INET_NUMHOOKS for this.
> 
> I'll submit a patch, that's for the heads up.

s/that's/thanks

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

end of thread, other threads:[~2020-10-14 13:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-14 12:59 iptables userspace API broken due to added value in nf_inet_hooks Jason A. Donenfeld
2020-10-14 13:01 ` Pablo Neira Ayuso
2020-10-14 13:01   ` Pablo Neira Ayuso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).