Re: [BUG] sg.c: sleeping function called from invalid context

On Thu, 2009-10-01 at 15:59 -0400, Douglas Gilbert wrote:
iceberg wrote:
KERNEL_VERSION: 2.6.31
DESCRIBE:
Driver sg.c might sleep in atomic context, because it calls
scsi_device_put under lock_kernel.

/drivers/scsi/sg.c:306:
static int
sg_open(struct inode *inode, struct file *filp)
{
...
lock_kernel();
...
error_out:
if (retval)
scsi_device_put(sdp->device);
...

Path to might_sleep macro from scsi_device_put:
1. scsi_device_put calls put_device at ./drivers/scsi/scsi.c:1111
2. put_device calls kobject_put at ./drivers/base/core.c:1038
3. kobject_put calls kref_put at ./lib/kobject.c
4. kref_put may call callback function kobject_release at ./lib/kref.c if
refcount becomes zero, which might_sleep because it calls user event. Details:
5.1 kobject_cleanup calls kobject_uevent at ./lib/kobject.c:555
5.2 kobject_uevent calls kobject_uevent_env at ./lib/kobject_uevent.c:282
5.3 kobject_uevent_env calls call_usermodehelper_exec at
include/linux/kmod.h:83
5.4 call_usermodehelper_exec calls wait_for_completion at ./kernel/kmod.c:481
5.5 wait_for_completion calls wait_for_common at ./kernel/sched.c:5710
5.6 wait_for_common calls might_sleep at ./kernels/sched.c:5692

Found by: Linux Driver Verification

This patch to sg_open() does one (and only one) unlock_kernel()
prior to scsi_device_put(). I presume sg_put_dev() may also
sleep so the unlock_kernel() is moved before it as well.

Hopefully Tomo will comment.

Really, this isn't a bug, so no fix is required.

The analysis is wrong on two levels. Firstly scsi_device_put() is
designed to be called from interrupt/locked context and secondly
lock_kernel isn't actually a lock taking interrupt context anyway. The
BLK is a strange beast; it's recursive and it's actually transparently
dropped and reacquired over schedule, so you can sleep by design with
the BKL held.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages

  • Re: NAPI poll routine happens in interrupt context?
    ... You will get more response to network issues on netdev@vger.kernel.org ... NAPI poll is usually called from softirq context. ... You shouldn't be calling things that could sleep! ... Hopefully we can bring this driver to such a state ...
    (Linux-Kernel)
  • Re: User vs. Worker threads
    ... is shut down or the thread terminates itself by calling ... Driver routines that run in a process context other than that of the system ... It has all the worker threads. ...
    (microsoft.public.win32.programmer.kernel)
  • Re: Question about FILE_OBJECT
    ... I think your missing something First the driver is an ndis intermediate ... I.E. every time they allocated a new context object they ...
    (microsoft.public.win32.programmer.kernel)
  • Re: Proposed new HOS rule
    ... The problem with current HOS, 12/12 HOS, and team driving is ... A rolling truck is almost the worst possible place to get quality sleep. ... A team driver under your plan would have to lie and claim his 4 or 5 hours ...
    (misc.transport.trucking)
  • Re: Problem with matching kind of NDIS driver.
    ... When it comes to outgoing packets, the reasons why his task is infeasible ... context of different threads. ... NDIS IM filter that is located immediately below ... We are telling you that in a NDIS Intermediate driver the process ...
    (microsoft.public.development.device.drivers)