Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> On Fri, Jul 06, 2018 at 02:53:25PM +0200, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> > > On Wed, Jul 04, 2018 at 09:13:06PM +0200, Florian Westphal wrote:
> > > > iptables-nft never requests these, but explicitly reject this.
> > > >
> > > > If it were requested, kernel will oops as ->target is NULL.
> > > >
> > > > Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
> > > > ---
> > > > net/netfilter/nft_compat.c | 12 ++++++++++++
> > > > 1 file changed, 12 insertions(+)
> > > >
> > > > diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
> > > > index 8d1ff654e5af..198cef9c2906 100644
> > > > --- a/net/netfilter/nft_compat.c
> > > > +++ b/net/netfilter/nft_compat.c
> > > > @@ -832,10 +832,16 @@ nft_target_select_ops(const struct nft_ctx *ctx,
> > > > rev = ntohl(nla_get_be32(tb[NFTA_TARGET_REV]));
> > > > family = ctx->family;
> > > >
> > > > + if (strcmp(tg_name, "ERROR") == 0)
> > > > + return ERR_PTR(-EINVAL);
> > >
> > > We can just reject XT_ERROR_TARGET and XT_STANDARD_TARGET here, to
> > > keep it simple, right? So we don't use this hardcoded "ERROR".
> >
> > Yes, that works too.
> > Rejecting XT_STANDARD_TARGET might be a good idea too in case someone
> > ever changes it to have a non-null evaluation function.
>
> Hm, I think we're already safe, right? I mean, the check for
> target->target to be NULL and return -EINVAL is sufficient.
>
> But I can just apply what I'm attaching for clarity if you prefer.
Rejecting from target_init should work.
I had placed everything in select_ops as that allocates the middleman
struct and I wanted to fail early.
The ebtables standard target ("standard", sigh) also lacks target eval
function so should fail because of ->target == NULL.
I'm only mentioning this because I think it should either strcmp
for XT_ERROR_TARGET only and handle rest via taget->target == NULL check,
or consider XT_STANDARD_TARGET, XT_ERROR_TARGET, "standard", and the
->target == NULL check.
As its slow path, I would go for the latter option.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
