netdev - Re: [PATCH net v5] bpf, net: Fix a potential race in do_sock

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: <CAADnVQKF=o6q2FzssEy9-jmye7+DB=S58KD8=dh=aRR5QTpJrA@mail.gmail.com>
Date: Sat, 31 Aug 2024 15:06:00 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Stanislav Fomichev <sdf@...ichev.me>
Cc: Tze-nan Wu <Tze-nan.Wu@...iatek.com>, Network Development <netdev@...r.kernel.org>, 
	bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>, 
	Bobule Chang (張弘義) <bobule.chang@...iatek.com>, 
	wsd_upstream <wsd_upstream@...iatek.com>, LKML <linux-kernel@...r.kernel.org>, 
	linux-mediatek@...ts.infradead.org, Kuniyuki Iwashima <kuniyu@...zon.com>, 
	Chen-Yao Chang (張禎耀) <chen-yao.chang@...iatek.com>, 
	Yanghui Li <yanghui.li@...iatek.com>, Cheng-Jui Wang <cheng-jui.wang@...iatek.com>, 
	Daniel Borkmann <daniel@...earbox.net>, John Fastabend <john.fastabend@...il.com>, 
	Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, 
	Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, KP Singh <kpsingh@...nel.org>, 
	Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Matthias Brugger <matthias.bgg@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, 
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH net v5] bpf, net: Fix a potential race in do_sock_getsockopt()

On Fri, Aug 30, 2024 at 8:29 PM Stanislav Fomichev <sdf@...ichev.me> wrote:
>
> On 08/30, Tze-nan Wu wrote:
> > There's a potential race when `cgroup_bpf_enabled(CGROUP_GETSOCKOPT)` is
> > false during the execution of `BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN`, but
> > becomes true when `BPF_CGROUP_RUN_PROG_GETSOCKOPT` is called.
> > This inconsistency can lead to `BPF_CGROUP_RUN_PROG_GETSOCKOPT` receiving
> > an "-EFAULT" from `__cgroup_bpf_run_filter_getsockopt(max_optlen=0)`.
> > Scenario shown as below:
> >
> >            `process A`                      `process B`
> >            -----------                      ------------
> >   BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN
> >                                             enable CGROUP_GETSOCKOPT
> >   BPF_CGROUP_RUN_PROG_GETSOCKOPT (-EFAULT)
> >
> > To resolve this, remove the `BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN` macro and
> > directly uses `copy_from_sockptr` to ensure that `max_optlen` is always
> > set before `BPF_CGROUP_RUN_PROG_GETSOCKOPT` is invoked.
> >
> > Fixes: 0d01da6afc54 ("bpf: implement getsockopt and setsockopt hooks")
> > Co-developed-by: Yanghui Li <yanghui.li@...iatek.com>
> > Signed-off-by: Yanghui Li <yanghui.li@...iatek.com>
> > Co-developed-by: Cheng-Jui Wang <cheng-jui.wang@...iatek.com>
> > Signed-off-by: Cheng-Jui Wang <cheng-jui.wang@...iatek.com>
> > Signed-off-by: Tze-nan Wu <Tze-nan.Wu@...iatek.com>
>
> Acked-by: Stanislav Fomichev <sdf@...ichev.me>

Considering it's rc6 I was debating whether it's net/bpf or -next
material, but could argue either way.

Tze-nan,
if I recall you were saying it affects android boot ?
If so please describe such details in the commit log next time.

Acked-by: Alexei Starovoitov <ast@...nel.org>

Kuba,
feel free to take it into net if you think it's an appropriate fix.