All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Christian L?hle <CLoehle@hyperstone.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"axboe@kernel.dk" <axboe@kernel.dk>
Subject: Re: [PATCH] block: prevent sending events from removed device
Date: Thu, 1 Jul 2021 08:42:04 +0100	[thread overview]
Message-ID: <YN1xzGPbx8ac8r3j@infradead.org> (raw)
In-Reply-To: <CWXP265MB2680EBAF4FEE64FBE80FAF25C4019@CWXP265MB2680.GBRP265.PROD.OUTLOOK.COM>

On Wed, Jun 30, 2021 at 08:09:39AM +0000, Christian L?hle wrote:
> Skip kobject_uevent_env in case the associated kobject
> no longer exists, as calling kobject_uevent_env with
> NULL is not safe.
> 

I don't see how this is going to work.  If the device is being
deleted the reference count will be zero and refcount_inc as
called from kobject_get will WARN.  We'll need to check the
disk is marked up, and we need to do that under a lock.

Something like this completely untested patch:

diff --git a/block/disk-events.c b/block/disk-events.c
index a75931ff5da4..27b845c51f2a 100644
--- a/block/disk-events.c
+++ b/block/disk-events.c
@@ -190,6 +190,9 @@ static void disk_check_events(struct disk_events *ev,
 
 	spin_unlock_irq(&ev->lock);
 
+	if (!(disk->flags & GENHD_FL_UP))
+		return;
+
 	/*
 	 * Tell userland about new events.  Only the events listed in
 	 * @disk->events are reported, and only if DISK_EVENT_FLAG_UEVENT
@@ -268,6 +271,8 @@ bool bdev_check_media_change(struct block_device *bdev)
 {
 	unsigned int events;
 
+	lockdep_assert_held(&bdev->bd_disk->open_mutex);
+
 	events = disk_clear_events(bdev->bd_disk, DISK_EVENT_MEDIA_CHANGE |
 				   DISK_EVENT_EJECT_REQUEST);
 	if (!(events & DISK_EVENT_MEDIA_CHANGE))
@@ -290,7 +295,10 @@ static void disk_events_workfn(struct work_struct *work)
 	struct delayed_work *dwork = to_delayed_work(work);
 	struct disk_events *ev = container_of(dwork, struct disk_events, dwork);
 
-	disk_check_events(ev, &ev->clearing);
+	mutex_lock(&ev->disk->open_mutex);
+	if (ev->disk->flags & GENHD_FL_UP)
+		disk_check_events(ev, &ev->clearing);
+	mutex_unlock(&ev->disk->open_mutex);
 }
 
 /*
diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index a093644ac39f..b8e77da44235 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -1735,8 +1735,10 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode,
 		/* invalidate the buffer track to force a reread */
 		BufferDrive = -1;
 		set_bit(drive, &fake_change);
+		mutex_lock(&bdev->bd_disk->open_mutex);
 		if (bdev_check_media_change(bdev))
 			floppy_revalidate(bdev->bd_disk);
+		mutex_unlock(&bdev->bd_disk->open_mutex);
 		return 0;
 	default:
 		return -EINVAL;
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 87460e0e5c72..2a97f22cfa0b 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -3185,8 +3185,10 @@ static int invalidate_drive(struct block_device *bdev)
 	/* invalidate the buffer track to force a reread */
 	set_bit((long)bdev->bd_disk->private_data, &fake_change);
 	process_fd_request();
+	mutex_lock(&bdev->bd_disk->open_mutex);
 	if (bdev_check_media_change(bdev))
 		floppy_revalidate(bdev->bd_disk);
+	mutex_unlock(&bdev->bd_disk->open_mutex);
 	return 0;
 }
 

  parent reply	other threads:[~2021-07-01  7:42 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-30  8:09 [PATCH] block: prevent sending events from removed device Christian Löhle
2021-06-30 11:46 ` [PATCHv2] " Christian Löhle
2021-07-01  7:42 ` Christoph Hellwig [this message]
2021-07-01  8:23   ` [PATCH] " Christian Löhle

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=YN1xzGPbx8ac8r3j@infradead.org \
    --to=hch@infradead.org \
    --cc=CLoehle@hyperstone.com \
    --cc=axboe@kernel.dk \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.