mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ben Hutchings <ben@decadent.org.uk>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: akpm@linux-foundation.org,
	"Linus Torvalds" <torvalds@linux-foundation.org>,
	"Jens Axboe" <axboe@kernel.dk>,
	"Mike Miller" <mike.miller@hp.com>,
	"Stephen M. Cameron" <scameron@beardog.cce.hp.com>
Subject: [74/83] cciss: fix broken mutex usage in ioctl
Date: Sun, 16 Jun 2013 23:01:38 +0100	[thread overview]
Message-ID: <lsq.1371420098.765426368@decadent.org.uk> (raw)
In-Reply-To: <lsq.1371420097.958132574@decadent.org.uk>

3.2.47-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: "Stephen M. Cameron" <scameron@beardog.cce.hp.com>

commit 03f47e888daf56c8e9046c674719a0bcc644eed5 upstream.

If a new logical drive is added and the CCISS_REGNEWD ioctl is invoked
(as is normal with the Array Configuration Utility) the process will
hang as below.  It attempts to acquire the same mutex twice, once in
do_ioctl() and once in cciss_unlocked_open().  The BKL was recursive,
the mutex isn't.

  Linux version 3.10.0-rc2 (scameron@localhost.localdomain) (gcc version 4.4.7 20120313 (Red Hat 4.4.7-3) (GCC) ) #1 SMP Fri May 24 14:32:12 CDT 2013
  [...]
  acu             D 0000000000000001     0  3246   3191 0x00000080
  Call Trace:
    schedule+0x29/0x70
    schedule_preempt_disabled+0xe/0x10
    __mutex_lock_slowpath+0x17b/0x220
    mutex_lock+0x2b/0x50
    cciss_unlocked_open+0x2f/0x110 [cciss]
    __blkdev_get+0xd3/0x470
    blkdev_get+0x5c/0x1e0
    register_disk+0x182/0x1a0
    add_disk+0x17c/0x310
    cciss_add_disk+0x13a/0x170 [cciss]
    cciss_update_drive_info+0x39b/0x480 [cciss]
    rebuild_lun_table+0x258/0x370 [cciss]
    cciss_ioctl+0x34f/0x470 [cciss]
    do_ioctl+0x49/0x70 [cciss]
    __blkdev_driver_ioctl+0x28/0x30
    blkdev_ioctl+0x200/0x7b0
    block_ioctl+0x3c/0x40
    do_vfs_ioctl+0x89/0x350
    SyS_ioctl+0xa1/0xb0
    system_call_fastpath+0x16/0x1b

This mutex usage was added into the ioctl path when the big kernel lock
was removed.  As it turns out, these paths are all thread safe anyway
(or can easily be made so) and we don't want ioctl() to be single
threaded in any case.

Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Mike Miller <mike.miller@hp.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/block/cciss.c |   32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -161,8 +161,6 @@ static irqreturn_t do_cciss_msix_intr(in
 static int cciss_open(struct block_device *bdev, fmode_t mode);
 static int cciss_unlocked_open(struct block_device *bdev, fmode_t mode);
 static int cciss_release(struct gendisk *disk, fmode_t mode);
-static int do_ioctl(struct block_device *bdev, fmode_t mode,
-		    unsigned int cmd, unsigned long arg);
 static int cciss_ioctl(struct block_device *bdev, fmode_t mode,
 		       unsigned int cmd, unsigned long arg);
 static int cciss_getgeo(struct block_device *bdev, struct hd_geometry *geo);
@@ -229,7 +227,7 @@ static const struct block_device_operati
 	.owner = THIS_MODULE,
 	.open = cciss_unlocked_open,
 	.release = cciss_release,
-	.ioctl = do_ioctl,
+	.ioctl = cciss_ioctl,
 	.getgeo = cciss_getgeo,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl = cciss_compat_ioctl,
@@ -1140,16 +1138,6 @@ static int cciss_release(struct gendisk
 	return 0;
 }
 
-static int do_ioctl(struct block_device *bdev, fmode_t mode,
-		    unsigned cmd, unsigned long arg)
-{
-	int ret;
-	mutex_lock(&cciss_mutex);
-	ret = cciss_ioctl(bdev, mode, cmd, arg);
-	mutex_unlock(&cciss_mutex);
-	return ret;
-}
-
 #ifdef CONFIG_COMPAT
 
 static int cciss_ioctl32_passthru(struct block_device *bdev, fmode_t mode,
@@ -1176,7 +1164,7 @@ static int cciss_compat_ioctl(struct blo
 	case CCISS_REGNEWD:
 	case CCISS_RESCANDISK:
 	case CCISS_GETLUNINFO:
-		return do_ioctl(bdev, mode, cmd, arg);
+		return cciss_ioctl(bdev, mode, cmd, arg);
 
 	case CCISS_PASSTHRU32:
 		return cciss_ioctl32_passthru(bdev, mode, cmd, arg);
@@ -1216,7 +1204,7 @@ static int cciss_ioctl32_passthru(struct
 	if (err)
 		return -EFAULT;
 
-	err = do_ioctl(bdev, mode, CCISS_PASSTHRU, (unsigned long)p);
+	err = cciss_ioctl(bdev, mode, CCISS_PASSTHRU, (unsigned long)p);
 	if (err)
 		return err;
 	err |=
@@ -1258,7 +1246,7 @@ static int cciss_ioctl32_big_passthru(st
 	if (err)
 		return -EFAULT;
 
-	err = do_ioctl(bdev, mode, CCISS_BIG_PASSTHRU, (unsigned long)p);
+	err = cciss_ioctl(bdev, mode, CCISS_BIG_PASSTHRU, (unsigned long)p);
 	if (err)
 		return err;
 	err |=
@@ -1308,11 +1296,14 @@ static int cciss_getpciinfo(ctlr_info_t
 static int cciss_getintinfo(ctlr_info_t *h, void __user *argp)
 {
 	cciss_coalint_struct intinfo;
+	unsigned long flags;
 
 	if (!argp)
 		return -EINVAL;
+	spin_lock_irqsave(&h->lock, flags);
 	intinfo.delay = readl(&h->cfgtable->HostWrite.CoalIntDelay);
 	intinfo.count = readl(&h->cfgtable->HostWrite.CoalIntCount);
+	spin_unlock_irqrestore(&h->lock, flags);
 	if (copy_to_user
 	    (argp, &intinfo, sizeof(cciss_coalint_struct)))
 		return -EFAULT;
@@ -1353,12 +1344,15 @@ static int cciss_setintinfo(ctlr_info_t
 static int cciss_getnodename(ctlr_info_t *h, void __user *argp)
 {
 	NodeName_type NodeName;
+	unsigned long flags;
 	int i;
 
 	if (!argp)
 		return -EINVAL;
+	spin_lock_irqsave(&h->lock, flags);
 	for (i = 0; i < 16; i++)
 		NodeName[i] = readb(&h->cfgtable->ServerName[i]);
+	spin_unlock_irqrestore(&h->lock, flags);
 	if (copy_to_user(argp, NodeName, sizeof(NodeName_type)))
 		return -EFAULT;
 	return 0;
@@ -1395,10 +1389,13 @@ static int cciss_setnodename(ctlr_info_t
 static int cciss_getheartbeat(ctlr_info_t *h, void __user *argp)
 {
 	Heartbeat_type heartbeat;
+	unsigned long flags;
 
 	if (!argp)
 		return -EINVAL;
+	spin_lock_irqsave(&h->lock, flags);
 	heartbeat = readl(&h->cfgtable->HeartBeat);
+	spin_unlock_irqrestore(&h->lock, flags);
 	if (copy_to_user(argp, &heartbeat, sizeof(Heartbeat_type)))
 		return -EFAULT;
 	return 0;
@@ -1407,10 +1404,13 @@ static int cciss_getheartbeat(ctlr_info_
 static int cciss_getbustypes(ctlr_info_t *h, void __user *argp)
 {
 	BusTypes_type BusTypes;
+	unsigned long flags;
 
 	if (!argp)
 		return -EINVAL;
+	spin_lock_irqsave(&h->lock, flags);
 	BusTypes = readl(&h->cfgtable->BusTypes);
+	spin_unlock_irqrestore(&h->lock, flags);
 	if (copy_to_user(argp, &BusTypes, sizeof(BusTypes_type)))
 		return -EFAULT;
 	return 0;


  parent reply	other threads:[~2013-06-16 22:09 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-16 22:01 [00/83] 3.2.47-rc1 review Ben Hutchings
2013-06-16 22:01 ` [19/83] xen-netback: don't disconnect frontend when seeing oversize packet Ben Hutchings
2013-06-16 22:01 ` [09/83] xfs: kill suid/sgid through the truncate path Ben Hutchings
2013-06-16 22:01 ` [17/83] xen-netback: fix sparse warning Ben Hutchings
2013-06-16 22:01 ` [10/83] iscsi-target: fix heap buffer overflow on error Ben Hutchings
2013-06-16 22:01 ` [04/83] cfg80211: check wdev->netdev in connection work Ben Hutchings
2013-06-16 22:01 ` [08/83] drm/radeon: fix card_posted check for newer asics Ben Hutchings
2013-06-16 22:01 ` [07/83] iwlwifi: dvm: fix zero LQ CMD sending avoidance Ben Hutchings
2013-06-16 22:01 ` [13/83] ALSA: usb-audio: fix possible hang and overflow in parse_uac2_sample_rate_range() Ben Hutchings
2013-06-16 22:01 ` [01/83] rapidio/tsi721: Fix interrupt mask when handling MSI Ben Hutchings
2013-06-16 22:01 ` [22/83] xen-netback: better names for thresholds Ben Hutchings
2013-06-16 22:01 ` [28/83] ext4: lock i_mutex when truncating orphan inodes Ben Hutchings
2013-06-17 10:18   ` Luis Henriques
2013-06-16 22:01 ` [21/83] xen-netback: avoid allocating variable size array on stack Ben Hutchings
2013-06-16 22:01 ` [02/83] ata_piix: add PCI IDs for Intel BayTail Ben Hutchings
2013-06-16 22:01 ` [06/83] mac80211: close AP_VLAN interfaces before unregistering all Ben Hutchings
2013-06-16 22:01 ` [14/83] ALSA: usb-audio: avoid integer overflow in create_fixed_stream_quirk() Ben Hutchings
2013-06-16 22:01 ` [12/83] tg3: Add read dma workaround for 5720 Ben Hutchings
2013-06-16 22:01 ` [25/83] jfs: fix a couple races Ben Hutchings
2013-06-16 22:01 ` [26/83] xen/events: Handle VIRQ_TIMER before any other hardirq in event loop Ben Hutchings
2013-06-16 22:01 ` [16/83] netback: remove redundant assignment Ben Hutchings
2013-06-16 22:01 ` [27/83] b43legacy: Fix crash on unload when firmware not available Ben Hutchings
2013-06-16 22:01 ` [30/83] ftrace: Move ftrace_filter_lseek out of CONFIG_DYNAMIC_FTRACE section Ben Hutchings
2013-06-16 22:01 ` [29/83] thinkpad-acpi: recognize latest V-Series using DMI_BIOS_VENDOR Ben Hutchings
2013-06-16 22:01 ` [24/83] xen-netfront: reduce gso_max_size to account for max TCP header Ben Hutchings
2013-06-16 22:01 ` [23/83] net: Add net_ratelimited_function and net_<level>_ratelimited macros Ben Hutchings
2013-06-16 22:01 ` [18/83] xen-netback: coalesce slots in TX path and fix regressions Ben Hutchings
2013-06-16 22:01 ` [11/83] tg3: Add New 5719 Read DMA workaround Ben Hutchings
2013-06-16 22:01 ` [15/83] xen-netback: remove skb in xen_netbk_alloc_page Ben Hutchings
2013-06-16 22:01 ` [20/83] xen-netback: remove redundent parameter in netbk_count_requests Ben Hutchings
2013-06-16 22:01 ` [03/83] libata: make ata_exec_internal_sg honor DMADIR Ben Hutchings
2013-06-16 22:01 ` [05/83] cifs: fix potential buffer overrun when composing a new options string Ben Hutchings
2013-06-16 22:01 ` [68/83] ath9k: Disable PowerSave by default Ben Hutchings
2013-06-17  9:55   ` Luis Henriques
2013-06-17 11:55     ` Ben Hutchings
2013-06-16 22:01 ` [56/83] USB: keyspan: fix bogus array index Ben Hutchings
2013-06-16 22:01 ` [55/83] drm/i915/sdvo: Use &intel_sdvo->ddc instead of intel_sdvo->i2c for DDC Ben Hutchings
2013-06-16 22:01 ` [73/83] audit: wait_for_auditd() should use TASK_UNINTERRUPTIBLE Ben Hutchings
2013-06-16 22:01 ` [66/83] drm/i915: prefer VBT modes for SVDO-LVDS over EDID Ben Hutchings
2013-06-16 22:01 ` [31/83] USB: serial: ftdi_sio: Handle the old_termios == 0 case e.g. uart_resume_port() Ben Hutchings
2013-06-16 22:01 ` [35/83] xhci: fix list access before init Ben Hutchings
2013-06-16 22:01 ` [58/83] ALSA: usb-audio - Fix invalid volume resolution on Logitech HD webcam c270 Ben Hutchings
2013-06-16 22:01 ` [60/83] ARM: 7743/1: compressed/head.S: work around new binutils warning Ben Hutchings
2013-06-16 22:01 ` [32/83] USB: ftdi_sio: Quiet sparse noise about using plain integer was NULL pointer Ben Hutchings
2013-06-16 22:01 ` [50/83] drm/i915: no lvds quirk for hp t5740 Ben Hutchings
2013-06-16 22:01 ` [51/83] radeon: Fix system hang issue when using KMS with older cards Ben Hutchings
2013-06-16 22:01 ` [71/83] CPU hotplug: provide a generic helper to disable/enable CPU hotplug Ben Hutchings
2013-06-16 22:01 ` [43/83] USB: iuu_phoenix: fix bulk-message timeout Ben Hutchings
2013-06-16 22:01 ` [48/83] ACPI / video: ignore BIOS initial backlight value for HP Pavilion g6 Ben Hutchings
2013-06-16 22:01 ` [63/83] hwmon: (adm1021) Strengthen chip detection for ADM1021, LM84 and MAX1617 Ben Hutchings
2013-06-16 22:01 ` [37/83] usb: dwc3: gadget: free trb pool only from epnum 2 Ben Hutchings
2013-06-16 22:01 ` [80/83] x86: Fix typo in kexec register clearing Ben Hutchings
2013-06-16 22:01 ` [52/83] USB: Serial: cypress_M8: Enable FRWD Dongle hidcom device Ben Hutchings
2013-06-16 22:01 ` [78/83] USB: spcp8x5: fix device initialisation at open Ben Hutchings
2013-06-16 22:01 ` Ben Hutchings [this message]
2013-06-16 22:01 ` [62/83] USB: option: blacklist network interface on Huawei E1820 Ben Hutchings
2013-06-16 22:01 ` [49/83] drm: fix a use-after-free when GPU acceleration disabled Ben Hutchings
2013-06-16 22:01 ` [64/83] drm/gma500/psb: Unpin framebuffer on crtc disable Ben Hutchings
2013-06-16 22:01 ` [46/83] ACPI video: ignore BIOS initial backlight value for HP 1000 Ben Hutchings
2013-06-16 22:01 ` [34/83] xhci-mem: init list heads at the beginning of init Ben Hutchings
2013-06-16 22:01 ` [65/83] drm/gma500/cdv: Unpin framebuffer on crtc disable Ben Hutchings
2013-06-16 22:01 ` [45/83] ACPI video: ignore BIOS backlight value for HP dm4 Ben Hutchings
2013-06-16 22:01 ` [44/83] USB: mos7720: fix message timeouts Ben Hutchings
2013-06-16 22:01 ` [40/83] USB: visor: fix initialisation of Treo/Kyocera devices Ben Hutchings
2013-06-16 22:01 ` [36/83] xhci - correct comp_mode_recovery_timer on return from hibernate Ben Hutchings
2013-06-16 22:01 ` [53/83] USB: serial: Add Option GTM681W to qcserial device table Ben Hutchings
2013-06-16 22:01 ` [33/83] drm/gma500: Increase max resolution for mode setting Ben Hutchings
2013-06-16 22:01 ` [82/83] tg3: Wait for boot code to finish after power on Ben Hutchings
2013-06-16 22:01 ` [57/83] USB: mos7720: fix hardware flow control Ben Hutchings
2013-06-16 22:01 ` [47/83] ACPI / video: ignore BIOS initial backlight value for HP m4 Ben Hutchings
2013-06-16 22:01 ` [59/83] ARM: 7742/1: topology: export cpu_topology Ben Hutchings
2013-06-16 22:01 ` [39/83] USB: serial: fix Treo/Kyocera interrrupt-in urb context Ben Hutchings
2013-06-16 22:01 ` [42/83] USB: ark3116: fix control-message timeout Ben Hutchings
2013-06-16 22:01 ` [61/83] USB: whiteheat: fix broken port configuration Ben Hutchings
2013-06-16 22:01 ` [75/83] drivers/rtc/rtc-twl.c: fix missing device_init_wakeup() when booted with device tree Ben Hutchings
2013-06-16 22:01 ` [81/83] md/raid1: consider WRITE as successful only if at least one non-Faulty and non-rebuilding drive completed it Ben Hutchings
2013-06-16 22:01 ` [38/83] USB: revert periodic scheduling bugfix Ben Hutchings
2013-06-16 22:01 ` [69/83] ath9k: Use minstrel rate control by default Ben Hutchings
2013-06-16 22:01 ` [79/83] USB: pl2303: fix device initialisation at open Ben Hutchings
2013-06-16 22:01 ` [83/83] powerpc: Fix emulation of illegal instructions on PowerNV platform Ben Hutchings
2013-06-16 22:01 ` [54/83] ALSA: usb-audio - Apply Logitech QuickCam Pro 9000 quirk only to audio iface Ben Hutchings
2013-06-16 22:01 ` [67/83] Bluetooth: Fix missing length checks for L2CAP signalling PDUs Ben Hutchings
2013-06-16 22:01 ` [72/83] reboot: rigrate shutdown/reboot to boot cpu Ben Hutchings
2013-06-16 22:01 ` [70/83] b43: stop format string leaking into error msgs Ben Hutchings
2013-06-16 22:01 ` [76/83] swap: avoid read_swap_cache_async() race to deadlock while waiting on discard I/O completion Ben Hutchings
2013-06-16 22:01 ` [77/83] mm: migration: add migrate_entry_wait_huge() Ben Hutchings
2013-06-16 22:01 ` [41/83] USB: mos7720: fix DMA to stack Ben Hutchings
2013-06-16 22:42 ` [00/83] 3.2.47-rc1 review Ben Hutchings

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=lsq.1371420098.765426368@decadent.org.uk \
    --to=ben@decadent.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mike.miller@hp.com \
    --cc=scameron@beardog.cce.hp.com \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®