Bug 194109 - [lor] if_lagg rmlock <-> if_addr_lock
Summary: [lor] if_lagg rmlock <-> if_addr_lock
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 11.0-STABLE
Hardware: Any Any
: --- Affects Many People
Assignee: Ryan Stone
URL: https://reviews.freebsd.org/D6845
Keywords: needs-qa, patch, regression
: 216731 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-10-03 08:21 UTC by Jean-Sébastien Pédron
Modified: 2017-02-15 17:38 UTC (History)
8 users (show)

See Also:
koobs: mfc-stable11?


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jean-Sébastien Pédron freebsd_committer 2014-10-03 08:21:31 UTC
The following LOR appeared between r272024 and r272450:

Oct  3 10:11:38 magellan kernel: lock order reversal:
Oct  3 10:11:38 magellan kernel: 1st 0xfffff80006c92008 if_lagg rmlock (if_lagg rmlock) @ /mnt/home/dumbbell/Projects/freebsd/src/SVN/head/sys/modules/if_lagg/../../net/if_lagg.c:1395
Oct  3 10:11:38 magellan kernel: 2nd 0xfffff80029f77990 if_addr_lock (if_addr_lock) @ /mnt/home/dumbbell/Projects/freebsd/src/SVN/head/sys/modules/if_lagg/../../net/if_lagg.c:1499
Oct  3 10:11:38 magellan kernel: KDB: stack backtrace:
Oct  3 10:11:38 magellan kernel: db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe012185e650
Oct  3 10:11:38 magellan kernel: kdb_backtrace() at kdb_backtrace+0x39/frame 0xfffffe012185e700
Oct  3 10:11:38 magellan kernel: witness_checkorder() at witness_checkorder+0xdc2/frame 0xfffffe012185e790
Oct  3 10:11:38 magellan kernel: _rw_wlock_cookie() at _rw_wlock_cookie+0x71/frame 0xfffffe012185e7d0
Oct  3 10:11:38 magellan kernel: lagg_ether_cmdmulti() at lagg_ether_cmdmulti+0x5c/frame 0xfffffe012185e810
Oct  3 10:11:38 magellan kernel: lagg_ioctl() at lagg_ioctl+0x114c/frame 0xfffffe012185e8f0
Oct  3 10:11:38 magellan kernel: in_control() at in_control+0x30b/frame 0xfffffe012185e970
Oct  3 10:11:38 magellan kernel: ifioctl() at ifioctl+0xba8/frame 0xfffffe012185ea30
Oct  3 10:11:38 magellan kernel: kern_ioctl() at kern_ioctl+0x22b/frame 0xfffffe012185ea90
Oct  3 10:11:38 magellan kernel: sys_ioctl() at sys_ioctl+0x13c/frame 0xfffffe012185eae0
Oct  3 10:11:38 magellan kernel: amd64_syscall() at amd64_syscall+0x25a/frame 0xfffffe012185ebf0
Oct  3 10:11:38 magellan kernel: Xfast_syscall() at Xfast_syscall+0xfb/frame 0xfffffe012185ebf0
Oct  3 10:11:38 magellan kernel: --- syscall (54, FreeBSD ELF64, sys_ioctl), rip = 0x8011df81a, rsp = 0x7fffffffe3e8, rbp = 0x7fffffffe460 ---
Comment 1 Jean-Sébastien Pédron freebsd_committer 2014-10-03 08:24:23 UTC
The lagg0 interface is configured like this in /etc/rc.conf:
ifconfig_lagg0="laggproto failover laggport re0 laggport wlan0 SYNCDHCP"
Comment 2 Navdeep Parhar freebsd_committer 2016-07-14 21:19:22 UTC
Deadlocks due to this LOR are easy to reproduce.  https://reviews.freebsd.org/D6845 has a proposed fix (not reviewed yet).  r272211 is mentioned as the possible culprit in that review.

This still occurs on 11.0-BETA1 so I've updated the version and added re@ to the CC list.
Comment 3 Glen Barber freebsd_committer 2016-08-08 16:21:35 UTC
I do not see a corresponding commit for this.  Is it still an issue?
Comment 4 Sean Bruno freebsd_committer 2016-08-08 16:27:32 UTC
(In reply to Glen Barber from comment #3)
No commit has been implemented for this issue.  My review/phab thing is a proof of concept that demonstrates that the LOR dissapears if you remove the LOCK, which is kind of dumb.

At this time there is no fix for this issue.
Comment 5 Alan Somers freebsd_committer 2016-11-11 15:01:15 UTC
I've got a better patch here, but it needs review
https://reviews.freebsd.org/D8306
Comment 6 Ryan Stone freebsd_committer 2016-11-11 17:46:22 UTC
The bug is not in the lagg driver but in the upper layer that held the if_addr_lock while calling into the driver.  This review should fix the lagg driver and any other ifnet implementation that needs to get a lock in its if_counter handler:

https://reviews.freebsd.org/D8498
Comment 7 commit-hook freebsd_committer 2016-11-12 19:03:42 UTC
A commit references this bug:

Author: rstone
Date: Sat Nov 12 19:03:24 UTC 2016
New revision: 308580
URL: https://svnweb.freebsd.org/changeset/base/308580

Log:
  Don't read if_counters with if_addr_lock held

  Calling into an ifnet implementation with the if_addr_lock already
  held can cause a LOR and potentially a deadlock, as ifnet
  implementations typically can take the if_addr_lock after their
  own locks during configuration.  Refactor a sysctl handler that
  was violating this to read if_counter data in a temporary buffer
  before the if_addr_lock is taken, and then copying the data
  in its final location later, when the if_addr_lock is held.

  PR: 194109
  Reported by: Jean-Sebastien Pedron
  MFC after: 2 weeks
  Differential Revision:	https://reviews.freebsd.org/D8498
  Reviewed by: sbruno

Changes:
  head/sys/net/rtsock.c
Comment 8 commit-hook freebsd_committer 2016-11-26 01:16:47 UTC
A commit references this bug:

Author: rstone
Date: Sat Nov 26 01:16:33 UTC 2016
New revision: 309177
URL: https://svnweb.freebsd.org/changeset/base/309177

Log:
  MFC r308580:

    Don't read if_counters with if_addr_lock held

    Calling into an ifnet implementation with the if_addr_lock already
    held can cause a LOR and potentially a deadlock, as ifnet
    implementations typically can take the if_addr_lock after their
    own locks during configuration.  Refactor a sysctl handler that
    was violating this to read if_counter data in a temporary buffer
    before the if_addr_lock is taken, and then copying the data
    in its final location later, when the if_addr_lock is held.

    PR: 194109
    Reported by: Jean-Sebastien Pedron
    MFC after: 2 weeks
    Differential Revision:        https://reviews.freebsd.org/D8498
    Reviewed by: sbruno

Changes:
_U  stable/11/
  stable/11/sys/net/rtsock.c
Comment 9 Alan Somers freebsd_committer 2017-02-15 17:38:24 UTC
*** Bug 216731 has been marked as a duplicate of this bug. ***