public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [Bug 12893] New: Race condition can cause two devices to get assigned the same device minor number.
@ 2009-03-18 21:18 bugme-daemon
  2009-03-18 21:27 ` James Bottomley
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: bugme-daemon @ 2009-03-18 21:18 UTC (permalink / raw)
  To: linux-scsi

http://bugzilla.kernel.org/show_bug.cgi?id=12893

           Summary: Race condition can cause two devices to get assigned the
                    same device minor number.
           Product: IO/Storage
           Version: 2.5
     KernelVersion: 2.6.27
          Platform: All
        OS/Version: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: SCSI
        AssignedTo: linux-scsi@vger.kernel.org
        ReportedBy: tdefeo@itsgames.com


Latest working kernel version:
Earliest failing kernel version:
Distribution:
Hardware Environment: x86
Software Environment:
Problem Description:
There is a race condition in scsi/sd.c caused bu the call to ida_get_new() not
being protected by a spinlock. If two devices appear at the same time (which
can happen when booting with multiple USB flash drives installed), occasionally
the timing will be just right such that two devices will get assigned the same
device minor number and hence the same device inode (i.e. /dev/sda). This
causes the scsi subsystem to crash.

Steps to reproduce: I can reproduce this by booting a system off of a USB flash
drive, with one or more other USB flash drives plugged in. It is sporadic,
depending on the timing, but it will eventually hang up when booting.

Here is a patch to add the proper locking and fix the problem:

--- ./linux-2.6.27.orignal/drivers/scsi/sd.c    2008-10-09 17:13:53.000000000
-0
500
+++ ./linux-2.6.27/drivers/scsi/sd.c    2009-03-18 14:19:42.000000000 -0600
@@ -99,6 +99,7 @@
 static void sd_print_sense_hdr(struct scsi_disk *, struct scsi_sense_hdr *);
 static void sd_print_result(struct scsi_disk *, int);

+static DEFINE_SPINLOCK(sda_index_lock); // tpd - 3/18/09 - Added.
 static DEFINE_IDA(sd_index_ida);

 /* This semaphore is used to mediate the 0->1 reference get in the
@@ -1808,8 +1809,9 @@
        do {
                if (!ida_pre_get(&sd_index_ida, GFP_KERNEL))
                        goto out_put;
-
+               spin_lock(&sda_index_lock); // tpd - 3/18/09 - Added.
                error = ida_get_new(&sd_index_ida, &index);
+               spin_unlock(&sda_index_lock); // tpd - 3/18/09 - Added.
        } while (error == -EAGAIN);

        if (error)
@@ -1883,7 +1885,9 @@
        return 0;

  out_free_index:
+       spin_lock(&sda_index_lock); // tpd - 3/18/09 - Added.
        ida_remove(&sd_index_ida, index);
+       spin_unlock(&sda_index_lock); // tpd - 3/18/09 - Added.
  out_put:
        put_disk(gd);
  out_free:
@@ -1933,7 +1937,9 @@
        struct scsi_disk *sdkp = to_scsi_disk(dev);
        struct gendisk *disk = sdkp->disk;

+       spin_lock(&sda_index_lock); // tpd - 3/18/09 - Added.
        ida_remove(&sd_index_ida, sdkp->index);
+       spin_unlock(&sda_index_lock); // tpd - 3/18/09 - Added.

        disk->private_data = NULL;
        put_disk(disk);


-- 
Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Bug 12893] New: Race condition can cause two devices to get assigned the same device minor number.
  2009-03-18 21:18 [Bug 12893] New: Race condition can cause two devices to get assigned the same device minor number bugme-daemon
@ 2009-03-18 21:27 ` James Bottomley
  2009-03-18 21:28 ` [Bug 12893] " bugme-daemon
  2009-03-19 11:42 ` bugme-daemon
  2 siblings, 0 replies; 4+ messages in thread
From: James Bottomley @ 2009-03-18 21:27 UTC (permalink / raw)
  To: bugme-daemon; +Cc: linux-scsi

On Wed, 2009-03-18 at 14:18 -0700, bugme-daemon@bugzilla.kernel.org
wrote:
> http://bugzilla.kernel.org/show_bug.cgi?id=12893
> 
>            Summary: Race condition can cause two devices to get assigned the
>                     same device minor number.
>            Product: IO/Storage
>            Version: 2.5
>      KernelVersion: 2.6.27
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: SCSI
>         AssignedTo: linux-scsi@vger.kernel.org
>         ReportedBy: tdefeo@itsgames.com
> 
> 
> Latest working kernel version:
> Earliest failing kernel version:
> Distribution:
> Hardware Environment: x86
> Software Environment:
> Problem Description:
> There is a race condition in scsi/sd.c caused bu the call to ida_get_new() not
> being protected by a spinlock. If two devices appear at the same time (which
> can happen when booting with multiple USB flash drives installed), occasionally
> the timing will be just right such that two devices will get assigned the same
> device minor number and hence the same device inode (i.e. /dev/sda). This
> causes the scsi subsystem to crash.
> 
> Steps to reproduce: I can reproduce this by booting a system off of a USB flash
> drive, with one or more other USB flash drives plugged in. It is sporadic,
> depending on the timing, but it will eventually hang up when booting.
> 
> Here is a patch to add the proper locking and fix the problem:
> 
> --- ./linux-2.6.27.orignal/drivers/scsi/sd.c    2008-10-09 17:13:53.000000000
> -0
> 500
> +++ ./linux-2.6.27/drivers/scsi/sd.c    2009-03-18 14:19:42.000000000 -0600
> @@ -99,6 +99,7 @@
>  static void sd_print_sense_hdr(struct scsi_disk *, struct scsi_sense_hdr *);
>  static void sd_print_result(struct scsi_disk *, int);
> 
> +static DEFINE_SPINLOCK(sda_index_lock); // tpd - 3/18/09 - Added.
>  static DEFINE_IDA(sd_index_ida);
> 
>  /* This semaphore is used to mediate the 0->1 reference get in the
> @@ -1808,8 +1809,9 @@
>         do {
>                 if (!ida_pre_get(&sd_index_ida, GFP_KERNEL))
>                         goto out_put;
> -
> +               spin_lock(&sda_index_lock); // tpd - 3/18/09 - Added.
>                 error = ida_get_new(&sd_index_ida, &index);
> +               spin_unlock(&sda_index_lock); // tpd - 3/18/09 - Added.
>         } while (error == -EAGAIN);
> 
>         if (error)
> @@ -1883,7 +1885,9 @@
>         return 0;
> 
>   out_free_index:
> +       spin_lock(&sda_index_lock); // tpd - 3/18/09 - Added.
>         ida_remove(&sd_index_ida, index);
> +       spin_unlock(&sda_index_lock); // tpd - 3/18/09 - Added.
>   out_put:
>         put_disk(gd);
>   out_free:
> @@ -1933,7 +1937,9 @@
>         struct scsi_disk *sdkp = to_scsi_disk(dev);
>         struct gendisk *disk = sdkp->disk;
> 
> +       spin_lock(&sda_index_lock); // tpd - 3/18/09 - Added.
>         ida_remove(&sd_index_ida, sdkp->index);
> +       spin_unlock(&sda_index_lock); // tpd - 3/18/09 - Added.
> 
>         disk->private_data = NULL;
>         put_disk(disk);

Very similar patch already upstream and in both stable kernels:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=4034cc68157bfa0b6622efe368488d3d3e20f4e6

James



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Bug 12893] Race condition can cause two devices to get assigned the same device minor number.
  2009-03-18 21:18 [Bug 12893] New: Race condition can cause two devices to get assigned the same device minor number bugme-daemon
  2009-03-18 21:27 ` James Bottomley
@ 2009-03-18 21:28 ` bugme-daemon
  2009-03-19 11:42 ` bugme-daemon
  2 siblings, 0 replies; 4+ messages in thread
From: bugme-daemon @ 2009-03-18 21:28 UTC (permalink / raw)
  To: linux-scsi

http://bugzilla.kernel.org/show_bug.cgi?id=12893





------- Comment #1 from anonymous@kernel-bugs.osdl.org  2009-03-18 14:28 -------
Reply-To: James.Bottomley@HansenPartnership.com

On Wed, 2009-03-18 at 14:18 -0700, bugme-daemon@bugzilla.kernel.org
wrote:
> http://bugzilla.kernel.org/show_bug.cgi?id=12893
> 
>            Summary: Race condition can cause two devices to get assigned the
>                     same device minor number.
>            Product: IO/Storage
>            Version: 2.5
>      KernelVersion: 2.6.27
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: SCSI
>         AssignedTo: linux-scsi@vger.kernel.org
>         ReportedBy: tdefeo@itsgames.com
> 
> 
> Latest working kernel version:
> Earliest failing kernel version:
> Distribution:
> Hardware Environment: x86
> Software Environment:
> Problem Description:
> There is a race condition in scsi/sd.c caused bu the call to ida_get_new() not
> being protected by a spinlock. If two devices appear at the same time (which
> can happen when booting with multiple USB flash drives installed), occasionally
> the timing will be just right such that two devices will get assigned the same
> device minor number and hence the same device inode (i.e. /dev/sda). This
> causes the scsi subsystem to crash.
> 
> Steps to reproduce: I can reproduce this by booting a system off of a USB flash
> drive, with one or more other USB flash drives plugged in. It is sporadic,
> depending on the timing, but it will eventually hang up when booting.
> 
> Here is a patch to add the proper locking and fix the problem:
> 
> --- ./linux-2.6.27.orignal/drivers/scsi/sd.c    2008-10-09 17:13:53.000000000
> -0
> 500
> +++ ./linux-2.6.27/drivers/scsi/sd.c    2009-03-18 14:19:42.000000000 -0600
> @@ -99,6 +99,7 @@
>  static void sd_print_sense_hdr(struct scsi_disk *, struct scsi_sense_hdr *);
>  static void sd_print_result(struct scsi_disk *, int);
> 
> +static DEFINE_SPINLOCK(sda_index_lock); // tpd - 3/18/09 - Added.
>  static DEFINE_IDA(sd_index_ida);
> 
>  /* This semaphore is used to mediate the 0->1 reference get in the
> @@ -1808,8 +1809,9 @@
>         do {
>                 if (!ida_pre_get(&sd_index_ida, GFP_KERNEL))
>                         goto out_put;
> -
> +               spin_lock(&sda_index_lock); // tpd - 3/18/09 - Added.
>                 error = ida_get_new(&sd_index_ida, &index);
> +               spin_unlock(&sda_index_lock); // tpd - 3/18/09 - Added.
>         } while (error == -EAGAIN);
> 
>         if (error)
> @@ -1883,7 +1885,9 @@
>         return 0;
> 
>   out_free_index:
> +       spin_lock(&sda_index_lock); // tpd - 3/18/09 - Added.
>         ida_remove(&sd_index_ida, index);
> +       spin_unlock(&sda_index_lock); // tpd - 3/18/09 - Added.
>   out_put:
>         put_disk(gd);
>   out_free:
> @@ -1933,7 +1937,9 @@
>         struct scsi_disk *sdkp = to_scsi_disk(dev);
>         struct gendisk *disk = sdkp->disk;
> 
> +       spin_lock(&sda_index_lock); // tpd - 3/18/09 - Added.
>         ida_remove(&sd_index_ida, sdkp->index);
> +       spin_unlock(&sda_index_lock); // tpd - 3/18/09 - Added.
> 
>         disk->private_data = NULL;
>         put_disk(disk);

Very similar patch already upstream and in both stable kernels:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=4034cc68157bfa0b6622efe368488d3d3e20f4e6

James


-- 
Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Bug 12893] Race condition can cause two devices to get assigned the same device minor number.
  2009-03-18 21:18 [Bug 12893] New: Race condition can cause two devices to get assigned the same device minor number bugme-daemon
  2009-03-18 21:27 ` James Bottomley
  2009-03-18 21:28 ` [Bug 12893] " bugme-daemon
@ 2009-03-19 11:42 ` bugme-daemon
  2 siblings, 0 replies; 4+ messages in thread
From: bugme-daemon @ 2009-03-19 11:42 UTC (permalink / raw)
  To: linux-scsi

http://bugzilla.kernel.org/show_bug.cgi?id=12893


alan@lxorguk.ukuu.org.uk changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |CLOSED
         Resolution|                            |PATCH_ALREADY_AVAILABLE




-- 
Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2009-03-19 11:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-18 21:18 [Bug 12893] New: Race condition can cause two devices to get assigned the same device minor number bugme-daemon
2009-03-18 21:27 ` James Bottomley
2009-03-18 21:28 ` [Bug 12893] " bugme-daemon
2009-03-19 11:42 ` bugme-daemon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox