IDE: Fix HDIO_DRIVE_RESET handling

Currently, the code path executing an HDIO_DRIVE_RESET ioctl is broken in various ways. Most importantly, it is treated as an out of band request in an illegal way which may very likely lead to system lock ups. Use the drive's request queue to avoid this problem (and fix a locking issue for free along the way). Signed-off-by: Elias Oltmanns <eo@nebensachen.de> Cc: "Alan Cox" <alan@lxorguk.ukuu.org.uk> Cc: "Randy Dunlap" <randy.dunlap@oracle.com> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

IDE: Fix HDIO_DRIVE_RESET handling
Currently, the code path executing an HDIO_DRIVE_RESET ioctl is broken in various ways. Most importantly, it is treated as an out of band request in an illegal way which may very likely lead to system lock ups. Use the drive's request queue to avoid this problem (and fix a locking issue for free along the way). Signed-off-by: Elias Oltmanns <eo@nebensachen.de> Cc: "Alan Cox" <alan@lxorguk.ukuu.org.uk> Cc: "Randy Dunlap" <randy.dunlap@oracle.com> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Elias Oltmanns · Bartlomiej Zolnierkiewicz
1 parent 72a3d651b2
Showing 4 changed files with 56 additions and 32 deletions Side-by-side Diff
drivers/ide/ide-io.c
drivers/ide/ide-iops.c
drivers/ide/ide.c
include/linux/ide.h
@@ -766,6 +766,18 @@
  	return ide_stopped;
 }
  
+static ide_startstop_t ide_special_rq(ide_drive_t *drive, struct request *rq)
+{
+	switch (rq->cmd[0]) {
+	case REQ_DRIVE_RESET:
+		return ide_do_reset(drive);
+	default:
+		blk_dump_rq_flags(rq, "ide_special_rq - bad request");
+		ide_end_request(drive, 0, 0);
+		return ide_stopped;
+	}
+}
+
 static void ide_check_pm_state(ide_drive_t *drive, struct request *rq)
 {
 	struct request_pm_state *pm = rq->data;
@@ -869,7 +881,16 @@
 			    pm->pm_step == ide_pm_state_completed)
 				ide_complete_pm_request(drive, rq);
 			return startstop;
-		}
+		} else if (!rq->rq_disk && blk_special_request(rq))
+			/*
+			 * TODO: Once all ULDs have been modified to
+			 * check for specific op codes rather than
+			 * blindly accepting any special request, the
+			 * check for ->rq_disk above may be replaced
+			 * by a more suitable mechanism or even
+			 * dropped entirely.
+			 */
+			return ide_special_rq(drive, rq);
  
 		drv = *(ide_driver_t **)rq->rq_disk->private_data;
 		return drv->do_request(drive, rq, block);
@@ -905,6 +905,14 @@
 }
 EXPORT_SYMBOL_GPL(ide_execute_pkt_cmd);
  
+static inline void ide_complete_drive_reset(ide_drive_t *drive)
+{
+	struct request *rq = drive->hwif->hwgroup->rq;
+
+	if (rq && blk_special_request(rq) && rq->cmd[0] == REQ_DRIVE_RESET)
+		ide_end_request(drive, 1, 0);
+}
+
 /* needed below */
 static ide_startstop_t do_reset1 (ide_drive_t *, int);
  
@@ -940,7 +948,7 @@
 	}
 	/* done polling */
 	hwgroup->polling = 0;
-	hwgroup->resetting = 0;
+	ide_complete_drive_reset(drive);
 	return ide_stopped;
 }
  
@@ -961,7 +969,7 @@
 		if (port_ops->reset_poll(drive)) {
 			printk(KERN_ERR "%s: host reset_poll failure for %s.\n",
 				hwif->name, drive->name);
-			return ide_stopped;
+			goto out;
 		}
 	}
  
@@ -1004,7 +1012,8 @@
 		}
 	}
 	hwgroup->polling = 0;	/* done polling */
-	hwgroup->resetting = 0; /* done reset attempt */
+out:
+	ide_complete_drive_reset(drive);
 	return ide_stopped;
 }
  
@@ -1090,7 +1099,6 @@
  
 	/* For an ATAPI device, first try an ATAPI SRST. */
 	if (drive->media != ide_disk && !do_not_try_atapi) {
-		hwgroup->resetting = 1;
 		pre_reset(drive);
 		SELECT_DRIVE(drive);
 		udelay (20);
  
@@ -1112,10 +1120,10 @@
  
 	if (io_ports->ctl_addr == 0) {
 		spin_unlock_irqrestore(&ide_lock, flags);
+		ide_complete_drive_reset(drive);
 		return ide_stopped;
 	}
  
-	hwgroup->resetting = 1;
 	/*
 	 * Note that we also set nIEN while resetting the device,
 	 * to mask unwanted interrupts from the interface during the reset.
@@ -529,6 +529,19 @@
 	return err;
 }
  
+static void generic_drive_reset(ide_drive_t *drive)
+{
+	struct request *rq;
+
+	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
+	rq->cmd_type = REQ_TYPE_SPECIAL;
+	rq->cmd_len = 1;
+	rq->cmd[0] = REQ_DRIVE_RESET;
+	rq->cmd_flags |= REQ_SOFTBARRIER;
+	blk_execute_rq(drive->queue, NULL, rq, 1);
+	blk_put_request(rq);
+}
+
 int generic_ide_ioctl(ide_drive_t *drive, struct file *file, struct block_device *bdev,
 			unsigned int cmd, unsigned long arg)
 {
  
@@ -603,33 +616,9 @@
 			if (!capable(CAP_SYS_ADMIN))
 				return -EACCES;
  
-			/*
-			 *	Abort the current command on the
-			 *	group if there is one, taking
-			 *	care not to allow anything else
-			 *	to be queued and to die on the
-			 *	spot if we miss one somehow
-			 */
-
-			spin_lock_irqsave(&ide_lock, flags);
-
-			if (HWGROUP(drive)->resetting) {
-				spin_unlock_irqrestore(&ide_lock, flags);
-				return -EBUSY;
-			}
-
-			ide_abort(drive, "drive reset");
-
-			BUG_ON(HWGROUP(drive)->handler);
-
-			/* Ensure nothing gets queued after we
-			   drop the lock. Reset will clear the busy */
-
-			HWGROUP(drive)->busy = 1;
-			spin_unlock_irqrestore(&ide_lock, flags);
-			(void) ide_do_reset(drive);
-
+			generic_drive_reset(drive);
 			return 0;
+
 		case HDIO_GET_BUSSTATE:
 			if (!capable(CAP_SYS_ADMIN))
 				return -EACCES;
@@ -139,6 +139,12 @@
 #define WAIT_MIN_SLEEP	(2*HZ/100)	/* 20msec - minimum sleep time */
  
 /*
+ * Op codes for special requests to be handled by ide_special_rq().
+ * Values should be in the range of 0x20 to 0x3f.
+ */
+#define REQ_DRIVE_RESET		0x20
+
+/*
  * Check for an interrupt and acknowledge the interrupt status
  */
 struct hwif_s;
...	...	@@ -766,6 +766,18 @@
766	766	return ide_stopped;
767	767	}
768	768
	769	+static ide_startstop_t ide_special_rq(ide_drive_t drive, struct request rq)
	770	+{
	771	+ switch (rq->cmd[0]) {
	772	+ case REQ_DRIVE_RESET:
	773	+ return ide_do_reset(drive);
	774	+ default:
	775	+ blk_dump_rq_flags(rq, "ide_special_rq - bad request");
	776	+ ide_end_request(drive, 0, 0);
	777	+ return ide_stopped;
	778	+ }
	779	+}
	780	+
769	781	static void ide_check_pm_state(ide_drive_t drive, struct request rq)
770	782	{
771	783	struct request_pm_state *pm = rq->data;
...	...	@@ -869,7 +881,16 @@
869	881	pm->pm_step == ide_pm_state_completed)
870	882	ide_complete_pm_request(drive, rq);
871	883	return startstop;
872		- }
	884	+ } else if (!rq->rq_disk && blk_special_request(rq))
	885	+ /*
	886	+ * TODO: Once all ULDs have been modified to
	887	+ * check for specific op codes rather than
	888	+ * blindly accepting any special request, the
	889	+ * check for ->rq_disk above may be replaced
	890	+ * by a more suitable mechanism or even
	891	+ * dropped entirely.
	892	+ */
	893	+ return ide_special_rq(drive, rq);
873	894
874	895	drv = (ide_driver_t *)rq->rq_disk->private_data;
875	896	return drv->do_request(drive, rq, block);
...	...	@@ -905,6 +905,14 @@
905	905	}
906	906	EXPORT_SYMBOL_GPL(ide_execute_pkt_cmd);
907	907
	908	+static inline void ide_complete_drive_reset(ide_drive_t *drive)
	909	+{
	910	+ struct request *rq = drive->hwif->hwgroup->rq;
	911	+
	912	+ if (rq && blk_special_request(rq) && rq->cmd[0] == REQ_DRIVE_RESET)
	913	+ ide_end_request(drive, 1, 0);
	914	+}
	915	+
908	916	/* needed below */
909	917	static ide_startstop_t do_reset1 (ide_drive_t *, int);
910	918
...	...	@@ -940,7 +948,7 @@
940	948	}
941	949	/* done polling */
942	950	hwgroup->polling = 0;
943		- hwgroup->resetting = 0;
	951	+ ide_complete_drive_reset(drive);
944	952	return ide_stopped;
945	953	}
946	954
...	...	@@ -961,7 +969,7 @@
961	969	if (port_ops->reset_poll(drive)) {
962	970	printk(KERN_ERR "%s: host reset_poll failure for %s.\n",
963	971	hwif->name, drive->name);
964		- return ide_stopped;
	972	+ goto out;
965	973	}
966	974	}
967	975
...	...	@@ -1004,7 +1012,8 @@
1004	1012	}
1005	1013	}
1006	1014	hwgroup->polling = 0; /* done polling */
1007		- hwgroup->resetting = 0; /* done reset attempt */
	1015	+out:
	1016	+ ide_complete_drive_reset(drive);
1008	1017	return ide_stopped;
1009	1018	}
1010	1019
...	...	@@ -1090,7 +1099,6 @@
1090	1099
1091	1100	/* For an ATAPI device, first try an ATAPI SRST. */
1092	1101	if (drive->media != ide_disk && !do_not_try_atapi) {
1093		- hwgroup->resetting = 1;
1094	1102	pre_reset(drive);
1095	1103	SELECT_DRIVE(drive);
1096	1104	udelay (20);
1097	1105
...	...	@@ -1112,10 +1120,10 @@
1112	1120
1113	1121	if (io_ports->ctl_addr == 0) {
1114	1122	spin_unlock_irqrestore(&ide_lock, flags);
	1123	+ ide_complete_drive_reset(drive);
1115	1124	return ide_stopped;
1116	1125	}
1117	1126
1118		- hwgroup->resetting = 1;
1119	1127	/*
1120	1128	* Note that we also set nIEN while resetting the device,
1121	1129	* to mask unwanted interrupts from the interface during the reset.
...	...	@@ -529,6 +529,19 @@
529	529	return err;
530	530	}
531	531
	532	+static void generic_drive_reset(ide_drive_t *drive)
	533	+{
	534	+ struct request *rq;
	535	+
	536	+ rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
	537	+ rq->cmd_type = REQ_TYPE_SPECIAL;
	538	+ rq->cmd_len = 1;
	539	+ rq->cmd[0] = REQ_DRIVE_RESET;
	540	+ rq->cmd_flags \|= REQ_SOFTBARRIER;
	541	+ blk_execute_rq(drive->queue, NULL, rq, 1);
	542	+ blk_put_request(rq);
	543	+}
	544	+
532	545	int generic_ide_ioctl(ide_drive_t drive, struct file file, struct block_device *bdev,
533	546	unsigned int cmd, unsigned long arg)
534	547	{
535	548
...	...	@@ -603,33 +616,9 @@
603	616	if (!capable(CAP_SYS_ADMIN))
604	617	return -EACCES;
605	618
606		- /*
607		- * Abort the current command on the
608		- * group if there is one, taking
609		- * care not to allow anything else
610		- * to be queued and to die on the
611		- * spot if we miss one somehow
612		- */
613		-
614		- spin_lock_irqsave(&ide_lock, flags);
615		-
616		- if (HWGROUP(drive)->resetting) {
617		- spin_unlock_irqrestore(&ide_lock, flags);
618		- return -EBUSY;
619		- }
620		-
621		- ide_abort(drive, "drive reset");
622		-
623		- BUG_ON(HWGROUP(drive)->handler);
624		-
625		- /* Ensure nothing gets queued after we
626		- drop the lock. Reset will clear the busy */
627		-
628		- HWGROUP(drive)->busy = 1;
629		- spin_unlock_irqrestore(&ide_lock, flags);
630		- (void) ide_do_reset(drive);
631		-
	619	+ generic_drive_reset(drive);
632	620	return 0;
	621	+
633	622	case HDIO_GET_BUSSTATE:
634	623	if (!capable(CAP_SYS_ADMIN))
635	624	return -EACCES;
...	...	@@ -139,6 +139,12 @@
139	139	#define WAIT_MIN_SLEEP (2HZ/100) / 20msec - minimum sleep time */
140	140
141	141	/*
	142	+ * Op codes for special requests to be handled by ide_special_rq().
	143	+ * Values should be in the range of 0x20 to 0x3f.
	144	+ */
	145	+#define REQ_DRIVE_RESET 0x20
	146	+
	147	+/*
142	148	* Check for an interrupt and acknowledge the interrupt status
143	149	*/
144	150	struct hwif_s;