2023-06-07 17:25:35

by Demi Marie Obenour

Subject: [PATCH] block: increment diskseq on all media change events

Currently, associating a loop device with a different file descriptor
does not increment its diskseq. This allows the following race
condition:

1. Program X opens a loop device
2. Program X gets the diskseq of the loop device.
3. Program X associates a file with the loop device.
4. Program X passes the loop device major, minor, and diskseq to
something.
5. Program X exits.
6. Program Y detaches the file from the loop device.
7. Program Y attaches a different file to the loop device.
8. The opener finally gets around to opening the loop device and checks
that the diskseq is what it expects it to be. Even though the
diskseq is the expected value, the result is that the opener is
accessing the wrong file.

From discussions with Christoph Hellwig, it appears that
disk_force_media_change() was supposed to call inc_diskseq(), but in
fact it does not. Adding a Fixes: tag to indicate this. Christoph's
Reported-by is because he stated that disk_force_media_change()
calls inc_diskseq(), which is what led me to discover that it should but
does not.

Reported-by: Christoph Hellwig <[email protected]>
Signed-off-by: Demi Marie Obenour <[email protected]>
Fixes: e6138dc12de9 ("block: add a helper to raise a media changed event")
Cc: [email protected] # 5.15+
---
block/disk-events.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/block/disk-events.c b/block/disk-events.c
index aee25a7e1ab7de8cc82b3c3774e83489d3a86ff9..450c2cbe23d56cc0fa8fa40db9866cdae0e7a626 100644
--- a/block/disk-events.c
+++ b/block/disk-events.c
@@ -307,6 +307,7 @@ bool disk_force_media_change(struct gendisk *disk, unsigned int events)
if (!(events & DISK_EVENT_MEDIA_CHANGE))
return false;

+ inc_diskseq(disk);
if (__invalidate_device(disk->part0, true))
pr_warn("VFS: busy inodes on changed media %s\n",
disk->disk_name);
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

2023-06-08 05:43:46

by Christoph Hellwig

Subject: Re: [PATCH] block: increment diskseq on all media change events

Looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

2023-06-20 13:29:38

by Jens Axboe

Subject: Re: [PATCH] block: increment diskseq on all media change events

On Wed, 07 Jun 2023 13:08:37 -0400, Demi Marie Obenour wrote:
> Currently, associating a loop device with a different file descriptor
> does not increment its diskseq. This allows the following race
> condition:
>
> 1. Program X opens a loop device
> 2. Program X gets the diskseq of the loop device.
> 3. Program X associates a file with the loop device.
> 4. Program X passes the loop device major, minor, and diskseq to
> something.
> 5. Program X exits.
> 6. Program Y detaches the file from the loop device.
> 7. Program Y attaches a different file to the loop device.
> 8. The opener finally gets around to opening the loop device and checks
> that the diskseq is what it expects it to be. Even though the
> diskseq is the expected value, the result is that the opener is
> accessing the wrong file.
>
> [...]

Applied, thanks!

[1/1] block: increment diskseq on all media change events
commit: b90ecc0379eb7bbe79337b0c7289390a98752646

Best regards,
--
Jens Axboe