netdev - Re: [PATCH net] net: clear the dst when changing skb protocol

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: <20250605070131.53d870f6@kernel.org>
Date: Thu, 5 Jun 2025 07:01:31 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Maciej Żenczykowski <maze@...gle.com>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
 pabeni@...hat.com, andrew+netdev@...n.ch, horms@...nel.org,
 martin.lau@...ux.dev, daniel@...earbox.net, john.fastabend@...il.com,
 eddyz87@...il.com, sdf@...ichev.me, haoluo@...gle.com, willemb@...gle.com,
 william.xuanziyang@...wei.com, alan.maguire@...cle.com, bpf@...r.kernel.org
Subject: Re: [PATCH net] net: clear the dst when changing skb protocol

On Thu, 5 Jun 2025 15:50:31 +0200 Maciej Żenczykowski wrote:
> On Thu, Jun 5, 2025 at 3:22 PM Jakub Kicinski <kuba@...nel.org> wrote:
> > > I wonder if this shouldn't drop dst even when doing ipv4->ipv4 or
> > > ipv6->ipv6 -- it's encapping, presumably old dst is irrelevant...  
> >
> > I keep going back and forth on this. You definitely have a point,
> > but I feel like there are levels to how BPF prog can make the dst
> > irrelevant:
> >  - change proto
> >  - encap
> >  - adjust room but not set any encap flag
> >  - overwrite the addrs without calling any helpers
> > First case we have to cover for safety, last we can't possibly cover.
> > So the question is whether we should draw the line somewhere in
> > the middle, or leave this patch as is and if the actual use case arrives
> > - let BPF call skb_dst_drop() as a kfunc. Right now I'm leaning towards
> > the latter.
> >
> > Does that make sense? Does anyone else have an opinion?  
> 
> It does make a fair bit of sense.
> Question: does calling it as a kfunc require kernel BTF?
> Specifically some ram limited devices want to disable CONFIG_DEBUG_INFO_BTF...
> I know normal bpf helpers don't need that...
> I guess you could always convert ipv4 -> ipv6 -> ipv4 ;-)

Not sure how BPF folks feel about that, but technically we could
also add a flag to bpf_skb_adjust_room() or bpf_skb_change_proto().