Bug 238707

Summary: [PATCH][LOR] Lock order reversal: rtentry vs "nd6 list"
Product: Base System Reporter: Arnaud Ysmal <arnaud.ysmal>
Component: kernAssignee: Alexander V. Chernikov <melifaro>
Status: New ---    
Severity: Affects Only Me CC: gbe, markj, melifaro
Priority: --- Keywords: patch
Version: CURRENT   
Hardware: Any   
OS: Any   
Description Flags
Patch to move the unlock of the rtentry just before the call of rt_notifydelete none

Description Arnaud Ysmal 2019-06-19 15:05:49 UTC
Created attachment 205221 [details]
Patch to move the unlock of the rtentry just before the call of rt_notifydelete


I encounter the following lor in the function rtrequest1_fib (in the RTM_DELETE case of the switch).

When deleting a rtentry, it is returned locked by rt_unlinkrte then during the call to rt_notifydelete the "nd6 list" lock is locked.

In the RTM_ADD switch case, the old_rt in unlink by rt_unlinkrte then unlocked and passed to the rt_notifydelete, with this I assume there is no issue to call rt_notifydelete with a rtentry unlocked.

The patch enclosed move the unlock of the rtentry just before the call of rt_notifydelete.

The witness trace is the following:
[2019-05-01 23:21:12] lock order reversal:
[2019-05-01 23:21:12] 1st 0xfffff80036683998 rtentry (rtentry) @ net/route.c:1230
[2019-05-01 23:21:12] 2nd 0xffffffff8147fe68 nd6 list (nd6 list) @ netinet6/nd6_rtr.c:545
[2019-05-01 23:21:12] stack backtrace:
[2019-05-01 23:21:12] #0 0xffffffff80544e10 at witness_debugger+0x70
[2019-05-01 23:21:12] #1 0xffffffff80544d10 at witness_checkorder+0xc90
[2019-05-01 23:21:12] #2 0xffffffff804ecfda at __rw_rlock+0x3a
[2019-05-01 23:21:12] #3 0xffffffff8068f798 at defrouter_lookup+0x28
[2019-05-01 23:21:12] #4 0xffffffff80689b40 at nd6_rtrequest+0x50
[2019-05-01 23:21:12] #5 0xffffffff805f4a66 at rtrequest1_fib+0x286
[2019-05-01 23:21:12] #6 0xffffffff805f89ff at route_output+0x77f
[2019-05-01 23:21:12] #7 0xffffffff8056ef7f at sosend_generic+0x43f
[2019-05-01 23:21:12] #8 0xffffffff8054f90d at soo_write+0x2d
[2019-05-01 23:21:12] #9 0xffffffff80548b0a at dofilewrite+0x8a
[2019-05-01 23:21:12] #10 0xffffffff805487e8 at kern_writev+0x68
[2019-05-01 23:21:12] #11 0xffffffff80548776 at sys_write+0x86
[2019-05-01 23:21:12] #12 0xffffffff80753bf9 at amd64_syscall+0x299
[2019-05-01 23:21:12] #13 0xffffffff80734bed at fast_syscall_common+0x101
Comment 1 Alexander V. Chernikov freebsd_committer 2019-06-19 23:31:13 UTC
Thank you for reporting the LOR and providing the patch!

The fix might be slightly more complicated, as nothing stops another thread changing the gateway of the same rte to change rt->rt_ifa so we would call the handler on the new if a instead of the old one. I'm testing a slightly different patch and will hopefully commit it tomorrow.
Comment 2 Arnaud Ysmal 2019-07-31 08:44:24 UTC
Is it me or ifa_request is only used by nd6 and only for the delete case?

By "slightly different patch" do you mean something like, completely removing ifa_rtrequest from the struct ifaddr?
Comment 3 Mark Johnston freebsd_committer 2020-06-02 14:24:18 UTC
(In reply to Alexander V. Chernikov from comment #1)
Alexander, has this been fixed in your recent work on the routing subsystem?
Comment 4 Alexander V. Chernikov freebsd_committer 2020-06-02 19:58:36 UTC
Maybe. Thanks for reminding me about this change, it slipped through the cracks.
I plan to remove ifa_rtequest callback by the end of this week and will ensure that this LOR won't happen anymore.