Subject: Re: [PATCH net 3/3] net/sched: act_ipt: zero skb->cb before calling target

From: Simon Horman <[email-masked]>
Date: Thu, 8 Jun 2023 12:45:38 +0200

On Wed, Jun 07, 2023 at 04:59:54PM +0200, Florian Westphal wrote:
> xtables relies on skb being owned by ip stack, i.e. with ipv4
> check in place skb->cb is supposed to be IPCB.
> 
> I don't see an immediate problem (REJECT target cannot be used anymore
> now that PRE/POSTROUTING hook validation has been fixed), but better be
> safe than sorry.
> 
> A much better patch would be to either mark act_ipt as
> "depends on BROKEN" or remove it altogether. I plan to do this
> for -next in the near future.
> 
> This tc extension is broken in the sense that tc lacks an
> equivalent of NF_STOLEN verdict.
> 
> With NF_STOLEN, target function takes complete ownership of skb, caller
> cannot dereference it anymore.
> 
> ACT_STOLEN cannot be used for this: it has a different meaning, caller
> is allowed to dereference the skb.
> 
> At this time NF_STOLEN won't be returned by any targets as far as I can
> see, but this may change in the future.
> 
> It might be possible to work around this via explcit list of allowed

nit: explcit -> explicit

> target extensions, that only return DROP or ACCEPT verdicts, but this
> seems to be error prone.
> 
> Existing selftest only validates xt_LOG and act_ipt is restricted
> to ipv4 so I don't think this action is used widely.
> 
> Signed-off-by: Florian Westphal <[email-masked]>

Reviewed-by: Simon Horman <[email-masked]>