Possible device resouce leak in nf_offload infra

Possible device resouce leak in nf_offload infra

[Florian Westphal]

Hi Pablo

net/netfilter/nf_dup_netdev.c :

 70 int nft_fwd_dup_netdev_offload(struct nft_offload_ctx *ctx,
 71                                struct nft_flow_rule *flow,
 72                                enum flow_action_id id, int oif)
 73 {
 74         struct flow_action_entry *entry;
 75         struct net_device *dev;
 76
 77         /* nft_flow_rule_destroy() releases the reference on this device. */

This comment is no longer true.

 78         dev = dev_get_by_index(ctx->net, oif);
 79         if (!dev)
 80                 return -EOPNOTSUPP;
 81
 82         entry = nft_flow_action_entry_next(ctx, flow);
 83         if (!entry)
 84                 return -E2BIG;

... because nft_flow_rule_destroy() cannot drop the device
ref when we return here, as dev is not assigned to entry
yet (and we got no entry).

AFAICS its safe to just swap this and have
lines 77/78 moved after line 82.

nft_fwd_dup_netdev_offload() could also use some debug
check to make sure this doesn't get called for actions
other than FLOW_ACTION_REDIRECT/FLOW_ACTION_MIRRED as
those are the only ones where nft_flow_rule_destroy() takes
action.

(or accessors and comments that say that accesses to the
 hidden union are illegal).

Is the analysis correct?  I can make a patch.

Re: Possible device resouce leak in nf_offload infra

[Pablo Neira Ayuso]

On Thu, Jun 04, 2026 at 09:17:42PM +0200, Florian Westphal wrote:
> Hi Pablo
> 
> net/netfilter/nf_dup_netdev.c :
> 
>  70 int nft_fwd_dup_netdev_offload(struct nft_offload_ctx *ctx,
>  71                                struct nft_flow_rule *flow,
>  72                                enum flow_action_id id, int oif)
>  73 {
>  74         struct flow_action_entry *entry;
>  75         struct net_device *dev;
>  76
>  77         /* nft_flow_rule_destroy() releases the reference on this device. */
> 
> This comment is no longer true.
> 
>  78         dev = dev_get_by_index(ctx->net, oif);
>  79         if (!dev)
>  80                 return -EOPNOTSUPP;
>  81
>  82         entry = nft_flow_action_entry_next(ctx, flow);
>  83         if (!entry)
>  84                 return -E2BIG;
> 
> ... because nft_flow_rule_destroy() cannot drop the device
> ref when we return here, as dev is not assigned to entry
> yet (and we got no entry).

Yes, it is a refcount leak in error path.

> AFAICS its safe to just swap this and have
> lines 77/78 moved after line 82.
> 
> nft_fwd_dup_netdev_offload() could also use some debug
> check to make sure this doesn't get called for actions
> other than FLOW_ACTION_REDIRECT/FLOW_ACTION_MIRRED as
> those are the only ones where nft_flow_rule_destroy() takes
> action.
> 
> (or accessors and comments that say that accesses to the
>  hidden union are illegal).
> 
> Is the analysis correct?  I can make a patch.

Yes, sure, go ahead. Thanks Florian