mbox series

[bpf,0/3] Block deletes from sockmap for tracing programs

Message ID 20240527-sockmap-verify-deletes-v1-0-944b372f2101@cloudflare.com (mailing list archive)
Headers show
Series Block deletes from sockmap for tracing programs | expand

Message

Jakub Sitnicki May 27, 2024, 11:20 a.m. UTC
We have seen a few syzkaller reports of locking violations triggered by
map_delete from sockmap/sockhash from an unexpected code path, for instance
when irqs were disabled, or during a kfree inside a map_update.

The consensus is [1] to block map_delete op in the verifier for programs
which are not allowed to update sockmap/sockhash already today, instead of
trying to make sockmap deletes lock-safe in every possible context.

[1] https://lore.kernel.org/r/87a5kfwe8l.fsf@cloudflare.com

---
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Hillf Danton <hdanton@sina.com>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>

---
Jakub Sitnicki (3):
      bpf: Allow delete from sockmap/sockhash only if update is allowed
      Revert "bpf, sockmap: Prevent lock inversion deadlock in map delete elem"
      selftests/bpf: Cover verifier checks for mutating sockmap/sockhash

 kernel/bpf/verifier.c                              |  10 +-
 net/core/sock_map.c                                |   6 -
 tools/testing/selftests/bpf/prog_tests/verifier.c  |   2 +
 .../selftests/bpf/progs/verifier_sockmap_mutate.c  | 187 +++++++++++++++++++++
 4 files changed, 196 insertions(+), 9 deletions(-)

Comments

John Fastabend May 27, 2024, 4:46 p.m. UTC | #1
Jakub Sitnicki wrote:
> We have seen a few syzkaller reports of locking violations triggered by
> map_delete from sockmap/sockhash from an unexpected code path, for instance
> when irqs were disabled, or during a kfree inside a map_update.
> 
> The consensus is [1] to block map_delete op in the verifier for programs
> which are not allowed to update sockmap/sockhash already today, instead of
> trying to make sockmap deletes lock-safe in every possible context.

+1 thanks Jakub. This makes sense to me I've never found a use case for
deleting socks from a tracing program.
patchwork-bot+netdevbpf@kernel.org May 27, 2024, 5:40 p.m. UTC | #2
Hello:

This series was applied to bpf/bpf.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Mon, 27 May 2024 13:20:06 +0200 you wrote:
> We have seen a few syzkaller reports of locking violations triggered by
> map_delete from sockmap/sockhash from an unexpected code path, for instance
> when irqs were disabled, or during a kfree inside a map_update.
> 
> The consensus is [1] to block map_delete op in the verifier for programs
> which are not allowed to update sockmap/sockhash already today, instead of
> trying to make sockmap deletes lock-safe in every possible context.
> 
> [...]

Here is the summary with links:
  - [bpf,1/3] bpf: Allow delete from sockmap/sockhash only if update is allowed
    https://git.kernel.org/bpf/bpf/c/98e948fb60d4
  - [bpf,2/3] Revert "bpf, sockmap: Prevent lock inversion deadlock in map delete elem"
    https://git.kernel.org/bpf/bpf/c/3b9ce0491a43
  - [bpf,3/3] selftests/bpf: Cover verifier checks for mutating sockmap/sockhash
    https://git.kernel.org/bpf/bpf/c/a63bf556160f

You are awesome, thank you!