HomeFreeBSD

bpf: Fix potential race conditions

Description

bpf: Fix potential race conditions

There're two possible race conditions,

  1. Concurrent bpfattach() and bpf_setif(), i.e., BIOCSETIF ioctl,
  2. Concurrent bpfdetach() and bpf_setif().

For the first case, userland may see BPF interface attached but it has
not been in the attached interfaces list bpf_iflist yet. Well it
will eventually be so this case does not matter.

For the second one, bpf_setif() may reference dead_bpf_if and the
kernel will panic (spotted by change [1], without the change we will
end up silently corrupted memory).

A simple fix could be that, we add additional check for dead_bpf_if
in the function bpf_setif(). But that requires to extend protection
of global lock (BPF_LOCK), i.e., BPF_LOCK should also protect the
assignment of ifp->if_bpf. That simple fix works but is apparently
not a good design. Since the attached interfaces list bpf_iflist is
the single source of truth, we look through it rather than check
against the interface's side, aka ifp->if_bpf.

This change has performance regression, that the cost of BPF interface
attach operation (BIOCSETIF ioctl) goes back from O(1) to O(N) (where
N is the number of BPF interfaces). Well we normally have sane amounts
of interfaces, an O(N) should be affordable.

[1] 7a974a649848 bpf: Make dead_bpf_if const

Fixes: 16d878cc99ef Fix the following bpf(4) race condition ...
MFC after: 4 days
Differential Revision: https://reviews.freebsd.org/D45725

Details

Provenance
zleiAuthored on Mon, Feb 3, 12:13 PM
Differential Revision
D45725: bpf: Fix potential race conditions
Parents
rGc7e0b94b7de7: riscv vmm: consider hart_mask_base argument in the SBI IPI handler.
Branches
Unknown
Tags
Unknown