[PATCH net v5] bpf, net: Fix a potential race in do_sock_getsockopt()

[Alexei Starovoitov]

On Fri, Aug 30, 2024 at 8:29 PM Stanislav Fomichev <sdf at fomichev.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 at mediatek.com>
> > Signed-off-by: Yanghui Li <yanghui.li at mediatek.com>
> > Co-developed-by: Cheng-Jui Wang <cheng-jui.wang at mediatek.com>
> > Signed-off-by: Cheng-Jui Wang <cheng-jui.wang at mediatek.com>
> > Signed-off-by: Tze-nan Wu <Tze-nan.Wu at mediatek.com>
>
> Acked-by: Stanislav Fomichev <sdf at fomichev.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 at kernel.org>

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