[PATCH nf] netfilter: nft_queue: only allow supported families

[PATCH nf] netfilter: nft_queue: only allow supported families

[Florian Westphal]

Trying to use 'queue' statement in ingress (for example)
triggers a splat on reinject:

WARNING: CPU: 3 PID: 1345 at net/netfilter/nf_queue.c:291

... because nf_reinject cannot find the ruleset head, so all
"reinject" attempts result in packet drop.

Ingress/egress do not support async resume at the moment anyway,
so disallow loading such rulesets with a more appropriate error
message.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nft_queue.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/net/netfilter/nft_queue.c b/net/netfilter/nft_queue.c
index 15e4b7640dc0..cb54a0a4b424 100644
--- a/net/netfilter/nft_queue.c
+++ b/net/netfilter/nft_queue.c
@@ -75,6 +75,24 @@ static const struct nla_policy nft_queue_policy[NFTA_QUEUE_MAX + 1] = {
 	[NFTA_QUEUE_SREG_QNUM]	= { .type = NLA_U32 },
 };
 
+static bool nft_queue_family_supported(const struct nft_ctx *ctx)
+{
+	switch (ctx->family) {
+	case NFPROTO_IPV4:
+	case NFPROTO_IPV6:
+	case NFPROTO_INET:
+	case NFPROTO_BRIDGE:
+		return true;
+	case NFPROTO_ARP:
+	case NFPROTO_DECNET:
+	case NFPROTO_NETDEV:
+	default:
+		break;
+	}
+
+	return false;
+}
+
 static int nft_queue_init(const struct nft_ctx *ctx,
 			  const struct nft_expr *expr,
 			  const struct nlattr * const tb[])
@@ -82,6 +100,9 @@ static int nft_queue_init(const struct nft_ctx *ctx,
 	struct nft_queue *priv = nft_expr_priv(expr);
 	u32 maxid;
 
+	if (!nft_queue_family_supported(ctx))
+		return -EOPNOTSUPP;
+
 	priv->queuenum = ntohs(nla_get_be16(tb[NFTA_QUEUE_NUM]));
 
 	if (tb[NFTA_QUEUE_TOTAL])
@@ -111,6 +132,9 @@ static int nft_queue_sreg_init(const struct nft_ctx *ctx,
 	struct nft_queue *priv = nft_expr_priv(expr);
 	int err;
 
+	if (!nft_queue_family_supported(ctx))
+		return -EOPNOTSUPP;
+
 	err = nft_parse_register_load(tb[NFTA_QUEUE_SREG_QNUM],
 				      &priv->sreg_qnum, sizeof(u32));
 	if (err < 0)
-- 
2.35.1

Re: [PATCH nf] netfilter: nft_queue: only allow supported families

[Pablo Neira Ayuso]

On Tue, Jul 26, 2022 at 12:43:48PM +0200, Florian Westphal wrote:
> Trying to use 'queue' statement in ingress (for example)
> triggers a splat on reinject:
> 
> WARNING: CPU: 3 PID: 1345 at net/netfilter/nf_queue.c:291
> 
> ... because nf_reinject cannot find the ruleset head, so all
> "reinject" attempts result in packet drop.
> 
> Ingress/egress do not support async resume at the moment anyway,
> so disallow loading such rulesets with a more appropriate error
> message.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  net/netfilter/nft_queue.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/net/netfilter/nft_queue.c b/net/netfilter/nft_queue.c
> index 15e4b7640dc0..cb54a0a4b424 100644
> --- a/net/netfilter/nft_queue.c
> +++ b/net/netfilter/nft_queue.c
> @@ -75,6 +75,24 @@ static const struct nla_policy nft_queue_policy[NFTA_QUEUE_MAX + 1] = {
>  	[NFTA_QUEUE_SREG_QNUM]	= { .type = NLA_U32 },
>  };
>  
> +static bool nft_queue_family_supported(const struct nft_ctx *ctx)
> +{
> +	switch (ctx->family) {
> +	case NFPROTO_IPV4:
> +	case NFPROTO_IPV6:
> +	case NFPROTO_INET:

there is a special inet/ingress, maybe it requires a sanity check here?

> +	case NFPROTO_BRIDGE:
> +		return true;
> +	case NFPROTO_ARP:
> +	case NFPROTO_DECNET:
> +	case NFPROTO_NETDEV:
> +	default:
> +		break;
> +	}
> +
> +	return false;
> +}
> +
>  static int nft_queue_init(const struct nft_ctx *ctx,
>  			  const struct nft_expr *expr,
>  			  const struct nlattr * const tb[])
> @@ -82,6 +100,9 @@ static int nft_queue_init(const struct nft_ctx *ctx,

Maybe .validate is a better place for this?

>  	struct nft_queue *priv = nft_expr_priv(expr);
>  	u32 maxid;
>  
> +	if (!nft_queue_family_supported(ctx))
> +		return -EOPNOTSUPP;
> +
>  	priv->queuenum = ntohs(nla_get_be16(tb[NFTA_QUEUE_NUM]));
>  
>  	if (tb[NFTA_QUEUE_TOTAL])
> @@ -111,6 +132,9 @@ static int nft_queue_sreg_init(const struct nft_ctx *ctx,
>  	struct nft_queue *priv = nft_expr_priv(expr);
>  	int err;
>  
> +	if (!nft_queue_family_supported(ctx))
> +		return -EOPNOTSUPP;
> +
>  	err = nft_parse_register_load(tb[NFTA_QUEUE_SREG_QNUM],
>  				      &priv->sreg_qnum, sizeof(u32));
>  	if (err < 0)
> -- 
> 2.35.1
> 

Re: [PATCH nf] netfilter: nft_queue: only allow supported families

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > +	case NFPROTO_INET:
> 
> there is a special inet/ingress, maybe it requires a sanity check here?

Right, this patch allows 'inet+ingress', whoch doesn't work either.

> >  static int nft_queue_init(const struct nft_ctx *ctx,
> >  			  const struct nft_expr *expr,
> >  			  const struct nlattr * const tb[])
> > @@ -82,6 +100,9 @@ static int nft_queue_init(const struct nft_ctx *ctx,
> 
> Maybe .validate is a better place for this?

Yep, I sent a v2.