diff mbox

[iproute] tc: m_xt: Fix segfault with iptables-1.6.0

Message ID 20170112142249.13041-1-phil@nwl.cc
State Accepted, archived
Delegated to: stephen hemminger
Headers show

Commit Message

Phil Sutter Jan. 12, 2017, 2:22 p.m. UTC
Said iptables version introduced struct xtables_globals field
'compat_rev', a function pointer. Initializing it is mandatory as
libxtables calls it without existence check.

Without this, tc segfaults when using the xt action like so:

| tc filter add dev d0 parent ffff: u32 match u32 0 0 \
|	action xt -j MARK --set-mark 20

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tc/m_xt.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Stephen Hemminger Jan. 12, 2017, 7:34 p.m. UTC | #1
On Thu, 12 Jan 2017 15:22:49 +0100
Phil Sutter <phil@nwl.cc> wrote:

> Said iptables version introduced struct xtables_globals field
> 'compat_rev', a function pointer. Initializing it is mandatory as
> libxtables calls it without existence check.
> 
> Without this, tc segfaults when using the xt action like so:
> 
> | tc filter add dev d0 parent ffff: u32 match u32 0 0 \
> |	action xt -j MARK --set-mark 20
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  tc/m_xt.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tc/m_xt.c b/tc/m_xt.c
> index dbb54981462ee..57ed40d7aa3a8 100644
> --- a/tc/m_xt.c
> +++ b/tc/m_xt.c
> @@ -77,6 +77,9 @@ static struct xtables_globals tcipt_globals = {
>  	.orig_opts = original_opts,
>  	.opts = original_opts,
>  	.exit_err = NULL,
> +#if (XTABLES_VERSION_CODE >= 11)
> +	.compat_rev = xtables_compatible_revision,
> +#endif
>  };
>  
>  /*

Ah the xtables API is not really an application friendly API by
any stretch of the imagination

Parenthesis not really necessary there.
Phil Sutter Jan. 12, 2017, 9:09 p.m. UTC | #2
On Thu, Jan 12, 2017 at 11:34:49AM -0800, Stephen Hemminger wrote:
> On Thu, 12 Jan 2017 15:22:49 +0100
> Phil Sutter <phil@nwl.cc> wrote:
> 
> > Said iptables version introduced struct xtables_globals field
> > 'compat_rev', a function pointer. Initializing it is mandatory as
> > libxtables calls it without existence check.
> > 
> > Without this, tc segfaults when using the xt action like so:
> > 
> > | tc filter add dev d0 parent ffff: u32 match u32 0 0 \
> > |	action xt -j MARK --set-mark 20
> > 
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >  tc/m_xt.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/tc/m_xt.c b/tc/m_xt.c
> > index dbb54981462ee..57ed40d7aa3a8 100644
> > --- a/tc/m_xt.c
> > +++ b/tc/m_xt.c
> > @@ -77,6 +77,9 @@ static struct xtables_globals tcipt_globals = {
> >  	.orig_opts = original_opts,
> >  	.opts = original_opts,
> >  	.exit_err = NULL,
> > +#if (XTABLES_VERSION_CODE >= 11)
> > +	.compat_rev = xtables_compatible_revision,
> > +#endif
> >  };
> >  
> >  /*
> 
> Ah the xtables API is not really an application friendly API by
> any stretch of the imagination

Yeah, this could have really been handled by the lib, especially since
it provides the callback to assign to it. But whatever, looking at the
git log shows it's not happening for the first time so at least it's
consistent. :)

> Parenthesis not really necessary there.

Oh, indeed! I just stuck to how the other XTABLES_VERSION_CODE checks
are made. Are you going to apply this patch either way? I'll then
follow-up dropping the parentheses at all places at once.

Thanks, Phil
diff mbox

Patch

diff --git a/tc/m_xt.c b/tc/m_xt.c
index dbb54981462ee..57ed40d7aa3a8 100644
--- a/tc/m_xt.c
+++ b/tc/m_xt.c
@@ -77,6 +77,9 @@  static struct xtables_globals tcipt_globals = {
 	.orig_opts = original_opts,
 	.opts = original_opts,
 	.exit_err = NULL,
+#if (XTABLES_VERSION_CODE >= 11)
+	.compat_rev = xtables_compatible_revision,
+#endif
 };
 
 /*