[bpf-next,v4,0/4] bpf: Don't EFAULT for {g,s}setsockopt with wrong optlen

Message

Stanislav Fomichev May 4, 2023, 6:43 p.m. UTC

optval larger than PAGE_SIZE leads to EFAULT if the BPF program
isn't careful enough. This is often overlooked and might break
completely unrelated socket options. Instead of EFAULT,
let's ignore BPF program buffer changes. See the first patch for
more info.

In addition, clearly document this corner case and reset optlen
in our selftests (in case somebody copy-pastes from them).

v4:
- ignore retval as well when optlen > PAGE_SIZE (Martin)

v3:
- don't hard-code PAGE_SIZE (Martin)
- reset orig_optlen in getsockopt when kernel part succeeds (Martin)

Stanislav Fomichev (4):
  bpf: Don't EFAULT for {g,s}setsockopt with wrong optlen
  selftests/bpf: Update EFAULT {g,s}etsockopt selftests
  selftests/bpf: Correctly handle optlen > 4096
  bpf: Document EFAULT changes for sockopt

 Documentation/bpf/prog_cgroup_sockopt.rst     |  57 ++++++++-
 kernel/bpf/cgroup.c                           |  15 +++
 .../bpf/prog_tests/cgroup_getset_retval.c     |  20 ++++
 .../selftests/bpf/prog_tests/sockopt.c        | 103 +++++++++++++++-
 .../bpf/prog_tests/sockopt_inherit.c          |  59 +++-------
 .../selftests/bpf/prog_tests/sockopt_multi.c  | 110 +++++-------------
 .../bpf/prog_tests/sockopt_qos_to_cc.c        |   2 +
 .../progs/cgroup_getset_retval_getsockopt.c   |  13 +++
 .../progs/cgroup_getset_retval_setsockopt.c   |  17 +++
 .../selftests/bpf/progs/sockopt_inherit.c     |  18 ++-
 .../selftests/bpf/progs/sockopt_multi.c       |  26 ++++-
 .../selftests/bpf/progs/sockopt_qos_to_cc.c   |  10 +-
 .../testing/selftests/bpf/progs/sockopt_sk.c  |  25 ++--
 13 files changed, 335 insertions(+), 140 deletions(-)

Comments

Martin KaFai Lau May 5, 2023, 10 p.m. UTC | #1

On 5/4/23 11:43 AM, Stanislav Fomichev wrote:
> optval larger than PAGE_SIZE leads to EFAULT if the BPF program
> isn't careful enough. This is often overlooked and might break
> completely unrelated socket options. Instead of EFAULT,
> let's ignore BPF program buffer changes. See the first patch for
> more info.
> 
> In addition, clearly document this corner case and reset optlen
> in our selftests (in case somebody copy-pastes from them).

Looks good. A respin is needed to address the selftest issues. The bpf CI will 
help to confirm that.

Looking forward to v5. Thanks.

Stanislav Fomichev May 5, 2023, 10:04 p.m. UTC | #2

On Fri, May 5, 2023 at 3:00 PM Martin KaFai Lau <[e-mail]> wrote:
>
> On 5/4/23 11:43 AM, Stanislav Fomichev wrote:
> > optval larger than PAGE_SIZE leads to EFAULT if the BPF program
> > isn't careful enough. This is often overlooked and might break
> > completely unrelated socket options. Instead of EFAULT,
> > let's ignore BPF program buffer changes. See the first patch for
> > more info.
> >
> > In addition, clearly document this corner case and reset optlen
> > in our selftests (in case somebody copy-pastes from them).
>
> Looks good. A respin is needed to address the selftest issues. The bpf CI will
> help to confirm that.
>
> Looking forward to v5. Thanks.

Thank you for the review, will take a look! The part about endianness
is surprising. Existing cases don't care because they are 1 byte; the
new ones are 4 and should, in theory, need the flipping.