Bug 220217
| Summary: | deadlock on enc and pf | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Kajetan Staszkiewicz <vegeta> | ||||
| Component: | kern | Assignee: | Andrey V. Elsukov <ae> | ||||
| Status: | Closed FIXED | ||||||
| Severity: | Affects Only Me | CC: | ae, kp, net | ||||
| Priority: | --- | Keywords: | patch | ||||
| Version: | 11.0-RELEASE | Flags: | koobs:
mfc-stable11+
|
||||
| Hardware: | amd64 | ||||||
| OS: | Any | ||||||
| Attachments: |
|
||||||
|
Description
Kajetan Staszkiewicz
2017-06-22 23:17:49 UTC
Do you also have the kernel log? I'd be interested in seeing the panic message. pf_socket_lookup() in the backtrace means you'll probably avoid the panic if you disable your UID related rules. Perhaps that can work as a temporary workaround. Under normal circumstances the kernel does not panic, it just goes 100% usage on bird or on interrupt. The packet which causes it is bird connecting to BGP router on the other side, at least on the dump I obtained from this particular crash.
I have the screenshot of crash with debug-enabled kernel but the stack trace is so long, that the original message is not visible anymore. Does kernel debugger console support scrolling? Pressing scroll-lock did not have any effect.
The line 411 where the error happens is:
372 __rw_rlock(volatile uintptr_t *c, const char *file, int line)
...
411 KASSERT(rw_wowner(rw) != curthread,
412 ("rw_rlock: wlock already held for %s @ %s:%d",
413 rw->lock_object.lo_name, file, line));
Can I obtain information about the other process holding the lock? (struct thread)*inp->inp_lock->rw_lock points to the same process "swi4: clock (0)". I know the live debugger has some helpers to show locks, but I would need to kill the machine again for that and set up serial console. I would prefer to use existing memory dump if possible.
(In reply to vegeta from comment #2) There is no other thread. We assert that 'rw_wowner(rw) != curthread', so the thread holding the lock must not be the current thread (trying to acquire the lock). That assertion fails, so we are the thread holding the lock, yet we try to lock again. That seems to be in tcp_timer_rexmt(). It holds a write lock on the struct inpcb, so when pf goes to look up the inpcb and tries to get a read lock we run into this assertion failure. (tcp_output() in fact asserts that the inpcb must be write locked when it's called.) I'm not quite sure how to fix this though. In fact, right now I don't understand how this ever works. (In reply to Kristof Provost from comment #3) > I'm not quite sure how to fix this though. In fact, right now I don't > understand how this ever works. I think we can extend ipsec_ctx_data structure by adding inpcb pointer. It will be initialized for IPSEC_ENC_BEFORE+HHOOK_TYPE_IPSEC_OUT case, and will be NULL for other cases. Then pass this pointer to the pfil_run_hooks(). In this case, I think, pf_test_rule() will not invoke pf_socket_lookup() due to pd->lookup.done = 1. And for other cases pf_socket_lookup() can be called, because we don't hold any inpcbs. Created attachment 183913 [details]
Proposed patch (untested)
Patch should be applicable to stable/11 and head/.
I'll try to find a moment to test it, hopefully next week. (In reply to Andrey V. Elsukov from comment #5) Thanks! I missed the inpcb being passed through netpfil. That explains why it usually works. I'm sorry, I did not have time to check the patch yet. Maybe next week. A commit references this bug: Author: ae Date: Mon Jul 31 11:04:36 UTC 2017 New revision: 321779 URL: https://svnweb.freebsd.org/changeset/base/321779 Log: Add inpcb pointer to struct ipsec_ctx_data and pass it to the pfil hook from enc_hhook(). This should solve the problem when pf is used with if_enc(4) interface, and outbound packet with existing PCB checked by pf, and this leads to deadlock due to pf does its own PCB lookup and tries to take rlock when wlock is already held. Now we pass PCB pointer if it is known to the pfil hook, this helps to avoid extra PCB lookup and thus rlock acquiring is not needed. For inbound packets it is safe to pass NULL, because we do not held any PCB locks yet. PR: 220217 MFC after: 3 weeks Sponsored by: Yandex LLC Changes: head/sys/net/if_enc.c head/sys/net/if_enc.h head/sys/netipsec/ipsec.h head/sys/netipsec/ipsec_input.c head/sys/netipsec/ipsec_output.c A commit references this bug: Author: ae Date: Mon Aug 21 09:03:21 UTC 2017 New revision: 322741 URL: https://svnweb.freebsd.org/changeset/base/322741 Log: MFC r321779: Add inpcb pointer to struct ipsec_ctx_data and pass it to the pfil hook from enc_hhook(). This should solve the problem when pf is used with if_enc(4) interface, and outbound packet with existing PCB checked by pf, and this leads to deadlock due to pf does its own PCB lookup and tries to take rlock when wlock is already held. Now we pass PCB pointer if it is known to the pfil hook, this helps to avoid extra PCB lookup and thus rlock acquiring is not needed. For inbound packets it is safe to pass NULL, because we do not held any PCB locks yet. PR: 220217 Sponsored by: Yandex LLC Changes: _U stable/11/ stable/11/sys/net/if_enc.c stable/11/sys/net/if_enc.h stable/11/sys/netipsec/ipsec.h stable/11/sys/netipsec/ipsec_input.c stable/11/sys/netipsec/ipsec_output.c Assign to committer that resolved |
