Page MenuHomeFreeBSD

Fix LOR with bpf detach
ClosedPublic

Authored by mmacy on Jul 1 2020, 3:14 AM.

Details

Summary

This fixes the following LOR

#0 0xffffffff80c32891 at witness_debugger+0x71
#1 0xffffffff80bd0377 at _sx_xlock+0x67
#2 0xffffffff80cf48bf at iflib_if_ioctl+0x2df
#3 0xffffffff80cd6227 at if_setflag+0xd7
#4 0xffffffff80cd60fa at ifpromisc+0x2a
#5 0xffffffff80ccac9b at bpf_detachd_locked+0x27b
#6 0xffffffff80ccd8f7 at bpf_dtor+0x87
#7 0xffffffff80a751bb at devfs_destroy_cdevpriv+0x9b
#8 0xffffffff80a78704 at devfs_close_f+0x64
#9 0xffffffff80b6b0da at _fdrop+0x1a
#10 0xffffffff80b6e24b at closef+0x1db
#11 0xffffffff80b6b5a6 at closefp+0x96
#12 0xffffffff8106b3f0 at amd64_syscall+0x140
#13 0xffffffff81040df0 at fast_syscall_common+0x101

Test Plan

run tcpdump on iflib driver interface, then press ^C

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mmacy requested review of this revision.Jul 1 2020, 3:14 AM

Why is it safe to just drop the lock?

What exactly is the LOR? I just see a stack.

Why is it safe to just drop the lock?

What exactly is the LOR? I just see a stack.

BPF detach holds its lock across detach calling in to set flags, here we're holding our lock calling in to attach - which AFAIK, doesn't touch iflib context.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 27 2020, 1:18 AM
This revision was automatically updated to reflect the committed changes.

I see the same LOR before and after the patch:

lock order reversal:
 1st 0xffffffff812a8ad8 bpf global lock (bpf global lock) @ /b/mnt/src/sys/net/bpf.c:799
 2nd 0xfffff801e05b5560 iflib ctx lock (iflib ctx lock) @ /b/mnt/src/sys/net/iflib.c:4122
stack backtrace:
#0 0xffffffff8069fb93 at witness_debugger+0x73
#1 0xffffffff8069f8e3 at witness_checkorder+0xab3
#2 0xffffffff8063b453 at _sx_xlock+0x73
#3 0xffffffff8075dba4 at iflib_if_ioctl+0x2f4
#4 0xffffffff80744d69 at if_setflag+0xd9
#5 0xffffffff80744c2c at ifpromisc+0x2c
#6 0xffffffff80739efc at bpf_detachd_locked+0x20c
#7 0xffffffff8073c93e at bpf_dtor+0x8e
#8 0xffffffff8050bc2b at devfs_destroy_cdevpriv+0x9b
#9 0xffffffff8050f2b5 at devfs_close_f+0x65
#10 0xffffffff805d37aa at _fdrop+0x1a
#11 0xffffffff805d6a5c at closef+0x1ec
#12 0xffffffff805d3d60 at closefp+0xa0
#13 0xffffffff809c4b88 at amd64_syscall+0x338

It's complaining about the CTX_LOCK(ctx); immediately following case SIOCSIFFLAGS:, not about holding the iflib ctx lock during IFDI_PROMISC_SET.