linux/drivers/staging/comedi
Ian Abbott b34aa86f12 staging: comedi: fix circular locking dependency in comedi_mmap()
Mmapping a comedi data buffer with lockdep checking enabled produced the
following kernel debug messages:

======================================================
[ INFO: possible circular locking dependency detected ]
3.5.0-rc3-ija1+ #9 Tainted: G         C
-------------------------------------------------------
comedi_test/4160 is trying to acquire lock:
 (&dev->mutex#2){+.+.+.}, at: [<ffffffffa00313f4>] comedi_mmap+0x57/0x1d9 [comedi]

but task is already holding lock:
 (&mm->mmap_sem){++++++}, at: [<ffffffff810c96fe>] vm_mmap_pgoff+0x41/0x76

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #1 (&mm->mmap_sem){++++++}:
       [<ffffffff8106d0e8>] lock_acquire+0x97/0x105
       [<ffffffff810ce3bc>] might_fault+0x6d/0x90
       [<ffffffffa0031ffb>] do_devinfo_ioctl.isra.7+0x11e/0x14c [comedi]
       [<ffffffffa003227f>] comedi_unlocked_ioctl+0x256/0xe48 [comedi]
       [<ffffffff810f7fcd>] vfs_ioctl+0x18/0x34
       [<ffffffff810f87fd>] do_vfs_ioctl+0x382/0x43c
       [<ffffffff810f88f9>] sys_ioctl+0x42/0x65
       [<ffffffff81415c62>] system_call_fastpath+0x16/0x1b

-> #0 (&dev->mutex#2){+.+.+.}:
       [<ffffffff8106c528>] __lock_acquire+0x101d/0x1591
       [<ffffffff8106d0e8>] lock_acquire+0x97/0x105
       [<ffffffff8140c894>] mutex_lock_nested+0x46/0x2a4
       [<ffffffffa00313f4>] comedi_mmap+0x57/0x1d9 [comedi]
       [<ffffffff810d5816>] mmap_region+0x281/0x492
       [<ffffffff810d5c92>] do_mmap_pgoff+0x26b/0x2a7
       [<ffffffff810c971a>] vm_mmap_pgoff+0x5d/0x76
       [<ffffffff810d493f>] sys_mmap_pgoff+0xc7/0x10d
       [<ffffffff81004d36>] sys_mmap+0x16/0x20
       [<ffffffff81415c62>] system_call_fastpath+0x16/0x1b

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(&mm->mmap_sem);
                               lock(&dev->mutex#2);
                               lock(&mm->mmap_sem);
  lock(&dev->mutex#2);

 *** DEADLOCK ***

To avoid the circular dependency, just try to get the lock in
`comedi_mmap()` instead of blocking.  Since the comedi device's main mutex
is heavily used, do a down-read of its `attach_lock` rwsemaphore
instead.  Trying to down-read `attach_lock` should only fail if
some task has down-write locked it, and that is only done while the
comedi device is being attached to or detached from a low-level hardware
device.

Unfortunately, acquiring the `attach_lock` doesn't prevent another
task replacing the comedi data buffer we are trying to mmap.  The
details of the buffer are held in a `struct comedi_buf_map` and pointed
to by `s->async->buf_map` where `s` is the comedi subdevice whose buffer
we are trying to map.  The `struct comedi_buf_map` is already reference
counted with a `struct kref`, so we can stop it being freed prematurely.

Modify `comedi_mmap()` to call new function
`comedi_buf_map_from_subdev_get()` to read the subdevice's current
buffer map pointer and increment its reference instead of accessing
`async->buf_map` directly.  Call `comedi_buf_map_put()` to decrement the
reference once the buffer map structure has been dealt with.  (Note that
`comedi_buf_map_put()` does nothing if passed a NULL pointer.)

`comedi_buf_map_from_subdev_get()` checks the subdevice's buffer map
pointer has been set and the buffer map has been initialized enough for
`comedi_mmap()` to deal with it (specifically, check the `n_pages`
member has been set to a non-zero value).  If all is well, the buffer
map's reference is incremented and a pointer to it is returned.  The
comedi subdevice's spin-lock is used to protect the checks.  Also use
the spin-lock in `__comedi_buf_alloc()` and `__comedi_buf_free()` to
protect changes to the subdevice's buffer map structure pointer and the
buffer map structure's `n_pages` member.  (This checking of `n_pages` is
a bit clunky and I [Ian Abbott] plan to deal with it in the future.)

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
Cc: <stable@vger.kernel.org> # 3.14.x, 3.15.x
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2014-04-16 11:41:45 -07:00
..
drivers staging: comedi: poc: remove obsolete driver 2014-03-20 01:57:01 +00:00
kcomedilib Staging: comedi: kcomedilib: replace deprecated simple_strtoul() with kstrtouint() 2014-02-15 12:42:41 -08:00
comedi_buf.c staging: comedi: fix circular locking dependency in comedi_mmap() 2014-04-16 11:41:45 -07:00
comedi_compat32.c file->f_op is never NULL... 2013-10-24 23:34:54 -04:00
comedi_compat32.h staging: comedi: remove FSF address from boilerplate text 2013-05-13 17:34:22 -04:00
comedi_fops.c staging: comedi: fix circular locking dependency in comedi_mmap() 2014-04-16 11:41:45 -07:00
comedi_internal.h staging: comedi: fix circular locking dependency in comedi_mmap() 2014-04-16 11:41:45 -07:00
comedi_pci.c staging: comedi: remove FSF address from boilerplate text 2013-05-13 17:34:22 -04:00
comedi_pcmcia.c staging: comedi: remove FSF address from boilerplate text 2013-05-13 17:34:22 -04:00
comedi_usb.c staging: comedi: introduce comedi_to_usb_dev() 2013-05-21 10:59:10 -07:00
comedi.h staging: comedi: remove FSF address from boilerplate text 2013-05-13 17:34:22 -04:00
comedidev.h Staging: comedi: Fix unnecessary space after function pointer 2014-02-27 08:38:12 -08:00
comedilib.h staging: comedi: comedi_bond: handle base channel for insn_bits 2013-08-26 06:41:56 -07:00
drivers.c staging: comedi: introduce comedi_timeout() 2014-02-14 09:28:43 -08:00
Kconfig staging: comedi: poc: remove obsolete driver 2014-03-20 01:57:01 +00:00
Makefile staging: comedi: remove comedi_debug module parameter 2013-11-25 11:50:53 -08:00
proc.c Staging: comedi: proc: title block update 2014-03-19 04:41:04 +00:00
range.c Staging: comedi: range: remove unnecessary sanity check 2014-03-08 18:59:48 -08:00
TODO MAINTAINERS: Update the list of maintainers for staging/comedi driver. 2013-07-24 09:51:18 -07:00