linux-kernel - Re: [PATCH 1/2] block: don't propagate unlisted DISK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 21 Apr 2011 19:46:16 +0200
From:	Jens Axboe <jaxboe@...ionio.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
CC:	Tejun Heo <tj@...nel.org>, Christoph Hellwig <hch@....de>,
	Neil Brown <neilb@...e.de>,
	"David S. Miller" <davem@...emloft.net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-ide@...r.kernel.org" <linux-ide@...r.kernel.org>,
	"kay.sievers@...y.org" <kay.sievers@...y.org>
Subject: Re: [PATCH 1/2] block: don't propagate unlisted DISK_EVENTs to  userland

On 2011-04-21 19:31, Jens Axboe wrote:
> On 2011-04-21 19:25, Linus Torvalds wrote:
>> Should I take these as patches, or through Jens/David/Who?
>>
>> I'll happily take them as patches, but I'd also like to hear
>> confirmation from the people who saw the lock-up that it's gone now..
> 
> I'm pulling them in now, with the elevator patch referenced in the IO
> scheduler switch email. I've verified that it fixes the ide-cd bug for
> me.

- Fix the oops on IO scheduler switch. Verified locally that the patch I
  proposed does fix the issue.

- The ide notification fixes from Tejun. Also verified locally to fix
  the boot hang on UP && !CONFIG_PREEMPT.

Please pull.


  git://git.kernel.dk/linux-2.6-block.git for-linus

Jens Axboe (1):
      elevator: check for ELEVATOR_INSERT_SORT_MERGE in !elvpriv case too

Tejun Heo (2):
      block: don't propagate unlisted DISK_EVENTs to userland
      ide: unexport DISK_EVENT_MEDIA_CHANGE for ide-gd and ide-cd

 block/elevator.c           |    3 ++-
 block/genhd.c              |    8 ++++++--
 drivers/ide/ide-cd.c       |    1 -
 drivers/ide/ide-cd_ioctl.c |    6 ++++++
 drivers/ide/ide-gd.c       |    7 ++++++-
 5 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/block/elevator.c b/block/elevator.c
index 6f6abc0..45ca1e3 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -671,7 +671,8 @@ void __elv_add_request(struct request_queue *q, struct request *rq, int where)
 			q->boundary_rq = rq;
 		}
 	} else if (!(rq->cmd_flags & REQ_ELVPRIV) &&
-		    where == ELEVATOR_INSERT_SORT)
+		    (where == ELEVATOR_INSERT_SORT ||
+		     where == ELEVATOR_INSERT_SORT_MERGE))
 		where = ELEVATOR_INSERT_BACK;
 
 	switch (where) {
diff --git a/block/genhd.c b/block/genhd.c
index b364bd0..2dd9887 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1588,9 +1588,13 @@ static void disk_events_workfn(struct work_struct *work)
 
 	spin_unlock_irq(&ev->lock);
 
-	/* tell userland about new events */
+	/*
+	 * Tell userland about new events.  Only the events listed in
+	 * @disk->events are reported.  Unlisted events are processed the
+	 * same internally but never get reported to userland.
+	 */
 	for (i = 0; i < ARRAY_SIZE(disk_uevents); i++)
-		if (events & (1 << i))
+		if (events & disk->events & (1 << i))
 			envp[nr_events++] = disk_uevents[i];
 
 	if (nr_events)
diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index fd1e117..a5ec5a7 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -1782,7 +1782,6 @@ static int ide_cd_probe(ide_drive_t *drive)
 	ide_cd_read_toc(drive, &sense);
 	g->fops = &idecd_ops;
 	g->flags |= GENHD_FL_REMOVABLE;
-	g->events = DISK_EVENT_MEDIA_CHANGE;
 	add_disk(g);
 	return 0;
 
diff --git a/drivers/ide/ide-cd_ioctl.c b/drivers/ide/ide-cd_ioctl.c
index 2a6bc50..02caa7d 100644
--- a/drivers/ide/ide-cd_ioctl.c
+++ b/drivers/ide/ide-cd_ioctl.c
@@ -79,6 +79,12 @@ int ide_cdrom_drive_status(struct cdrom_device_info *cdi, int slot_nr)
 	return CDS_DRIVE_NOT_READY;
 }
 
+/*
+ * ide-cd always generates media changed event if media is missing, which
+ * makes it impossible to use for proper event reporting, so disk->events
+ * is cleared to 0 and the following function is used only to trigger
+ * revalidation and never propagated to userland.
+ */
 unsigned int ide_cdrom_check_events_real(struct cdrom_device_info *cdi,
 					 unsigned int clearing, int slot_nr)
 {
diff --git a/drivers/ide/ide-gd.c b/drivers/ide/ide-gd.c
index c4ffd48..70ea876 100644
--- a/drivers/ide/ide-gd.c
+++ b/drivers/ide/ide-gd.c
@@ -298,6 +298,12 @@ static unsigned int ide_gd_check_events(struct gendisk *disk,
 		return 0;
 	}
 
+	/*
+	 * The following is used to force revalidation on the first open on
+	 * removeable devices, and never gets reported to userland as
+	 * genhd->events is 0.  This is intended as removeable ide disk
+	 * can't really detect MEDIA_CHANGE events.
+	 */
 	ret = drive->dev_flags & IDE_DFLAG_MEDIA_CHANGED;
 	drive->dev_flags &= ~IDE_DFLAG_MEDIA_CHANGED;
 
@@ -413,7 +419,6 @@ static int ide_gd_probe(ide_drive_t *drive)
 	if (drive->dev_flags & IDE_DFLAG_REMOVABLE)
 		g->flags = GENHD_FL_REMOVABLE;
 	g->fops = &ide_gd_ops;
-	g->events = DISK_EVENT_MEDIA_CHANGE;
 	add_disk(g);
 	return 0;
 
> 


-- 
Jens Axboe

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