linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <greg@kroah.com>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org,
	"Stephen M. Cameron" <scameron@beardog.cce.hp.com>,
	Jens Axboe <axboe@kernel.dk>, Mike Miller <mike.miller@hp.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: [ 13/26] cciss: fix broken mutex usage in ioctl
Date: Tue, 18 Jun 2013 09:14:53 -0700	[thread overview]
Message-ID: <20130618161235.546425531@linuxfoundation.org> (raw)
In-Reply-To: <20130618161231.154881788@linuxfoundation.org>

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

3.4-stable 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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 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-18 16:37 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-18 16:14 [ 00/26] 3.4.50-stable review Greg Kroah-Hartman
2013-06-18 16:14 ` [ 01/26] b43: stop format string leaking into error msgs Greg Kroah-Hartman
2013-06-18 16:14 ` [ 02/26] libceph: must hold mutex for reset_changed_osds() Greg Kroah-Hartman
2013-06-18 16:14 ` [ 03/26] ceph: add cpu_to_le32() calls when encoding a reconnect capability Greg Kroah-Hartman
2013-06-18 16:14 ` [ 04/26] ceph: ceph_pagelist_append might sleep while atomic Greg Kroah-Hartman
2013-06-18 16:14 ` [ 05/26] drivers/rtc/rtc-twl.c: fix missing device_init_wakeup() when booted with device tree Greg Kroah-Hartman
2013-06-18 16:14 ` [ 06/26] drm/gma500/psb: Unpin framebuffer on crtc disable Greg Kroah-Hartman
2013-06-18 16:14 ` [ 07/26] drm/gma500/cdv: " Greg Kroah-Hartman
2013-06-18 16:14 ` [ 08/26] Bluetooth: Fix mgmt handling of power on failures Greg Kroah-Hartman
2013-06-18 16:14 ` [ 09/26] ath9k: Disable PowerSave by default Greg Kroah-Hartman
2013-06-18 16:14 ` [ 10/26] ath9k: Use minstrel rate control " Greg Kroah-Hartman
2013-06-18 16:14 ` [ 11/26] CPU hotplug: provide a generic helper to disable/enable CPU hotplug Greg Kroah-Hartman
2013-06-18 16:14 ` [ 12/26] reboot: rigrate shutdown/reboot to boot cpu Greg Kroah-Hartman
2013-06-18 16:14 ` Greg Kroah-Hartman [this message]
2013-06-18 16:14 ` [ 14/26] drm/i915: prefer VBT modes for SVDO-LVDS over EDID Greg Kroah-Hartman
2013-06-18 16:14 ` [ 15/26] swap: avoid read_swap_cache_async() race to deadlock while waiting on discard I/O completion Greg Kroah-Hartman
2013-06-18 16:14 ` [ 16/26] md/raid1: consider WRITE as successful only if at least one non-Faulty and non-rebuilding drive completed it Greg Kroah-Hartman
2013-06-18 16:14 ` [ 17/26] mm: migration: add migrate_entry_wait_huge() Greg Kroah-Hartman
2013-06-18 16:14 ` [ 18/26] x86: Fix typo in kexec register clearing Greg Kroah-Hartman
2013-06-18 16:14 ` [ 19/26] libceph: clear messenger auth_retry flag when we authenticate Greg Kroah-Hartman
2013-06-18 16:15 ` [ 20/26] libceph: fix authorizer invalidation Greg Kroah-Hartman
2013-06-18 16:15 ` [ 21/26] libceph: add update_authorizer auth method Greg Kroah-Hartman
2013-06-18 16:15 ` [ 22/26] libceph: wrap auth ops in wrapper functions Greg Kroah-Hartman
2013-06-18 16:15 ` [ 23/26] libceph: wrap auth methods in a mutex Greg Kroah-Hartman
2013-06-18 16:15 ` [ 24/26] ceph: fix statvfs fr_size Greg Kroah-Hartman
2013-06-18 16:15 ` [ 25/26] powerpc: Fix stack overflow crash in resume_kernel when ftracing Greg Kroah-Hartman
2013-06-18 16:15 ` [ 26/26] powerpc: Fix missing/delayed calls to irq_work Greg Kroah-Hartman
     [not found] ` <CAKocOOOnfMbYymOamd8Woy=tmjnQ1i9xGLx-U2Unj8j7sk2RQA@mail.gmail.com>
2013-06-18 22:23   ` [ 00/26] 3.4.50-stable review Shuah Khan
2013-06-20 10:53 ` Satoru Takeuchi
2013-06-20 16:59   ` Greg Kroah-Hartman

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=20130618161235.546425531@linuxfoundation.org \
    --to=greg@kroah.com \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=gregkh@linuxfoundation.org \
    --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;
as well as URLs for NNTP newsgroup(s).