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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZIGxUtaYoRl50Ldv@corigine.com>
Date: Thu, 8 Jun 2023 12:45:38 +0200
From: Simon Horman <simon.horman@...igine.com>
To: Florian Westphal <fw@...len.de>
Cc: netdev@...r.kernel.org, jhs@...atatu.com, xiyou.wangcong@...il.com,
	jiri@...nulli.us
Subject: Re: [PATCH net 3/3] net/sched: act_ipt: zero skb->cb before calling
 target

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 <fw@...len.de>

Reviewed-by: Simon Horman <simon.horman@...igine.com>