netdev - Re: [PATCH net] tun: Remove nested call to bpf_net_ctx_set() in do_xdp

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20240725121506.15501-1-aha310510@gmail.com>
Date: Thu, 25 Jul 2024 21:15:06 +0900
From: Jeongjun Park <aha310510@...il.com>
To: pabeni@...hat.com
Cc: aha310510@...il.com,
	bigeasy@...utronix.de,
	bpf@...r.kernel.org,
	davem@...emloft.net,
	edumazet@...gle.com,
	jasowang@...hat.com,
	jiri@...nulli.us,
	kuba@...nel.org,
	linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org,
	syzbot+44623300f057a28baf1e@...kaller.appspotmail.com,
	syzkaller-bugs@...glegroups.com,
	willemdebruijn.kernel@...il.com
Subject: Re: [PATCH net] tun: Remove nested call to bpf_net_ctx_set() in do_xdp_generic()

Paolo Abeni wrote:
>
> On 7/25/24 04:43, Willem de Bruijn wrote:
> > Jeongjun Park wrote:
> >> In the previous commit, bpf_net_context handling was added to
> >> tun_sendmsg() and do_xdp_generic(), but if you write code like this,
> >> bpf_net_context overlaps in the call trace below, causing various
> >> memory corruptions.
> >
> > I'm no expert on this code, but commit 401cb7dae813 that introduced
> > bpf_net_ctx_set explicitly states that nested calls are allowed.
> >
> > And the function does imply that:
> >
> > static inline struct bpf_net_context *bpf_net_ctx_set(struct bpf_net_context *bpf_net_ctx)
> > {
> >          struct task_struct *tsk = current;
> >
> >          if (tsk->bpf_net_context != NULL)
> >                  return NULL;
> >          bpf_net_ctx->ri.kern_flags = 0;
> >
> >          tsk->bpf_net_context = bpf_net_ctx;
> >          return bpf_net_ctx;
> > }
>
> I agree with Willem, the ctx nesting looks legit generally speaking.
> @Jeongjun: you need to track down more accurately the issue root cause
> and include such info into the commit message.
>
> Skimming over the code I *think* do_xdp_generic() is not cleaning the
> nested context in all the paths before return and that could cause the
> reported issue.

Thanks to your comment, I re-read the code and found the root cause.
I will send a patch for that bug.

Regards,
Jeongjun Park