From: Josef Bacik jbacik@fb.com
This fixes a use after free bug, we need to do the del_gendisk after we cleanup the queue on the device.
Fixes: c6a4759ea0c9 ("nbd: add device refcounting") cc: stable@vger.kernel.org Signed-off-by: Josef Bacik jbacik@fb.com --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 86258b00a1d4..e33da3e6aa20 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -174,8 +174,8 @@ static void nbd_dev_remove(struct nbd_device *nbd) { struct gendisk *disk = nbd->disk; if (disk) { - del_gendisk(disk); blk_cleanup_queue(disk->queue); + del_gendisk(disk); blk_mq_free_tag_set(&nbd->tag_set); disk->private_data = NULL; put_disk(disk);
From: Josef Bacik jbacik@fb.com
I messed up changing the size of an NBD device while it was connected by not actually updating the device or doing the uevent. Fix this by updating everything if we're connected and we change the size.
cc: stable@vger.kernel.org Fixes: 639812a ("nbd: don't set the device size until we're connected") Signed-off-by: Josef Bacik jbacik@fb.com --- drivers/block/nbd.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index e33da3e6aa20..1520383b12f6 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -243,6 +243,8 @@ static void nbd_size_set(struct nbd_device *nbd, loff_t blocksize, struct nbd_config *config = nbd->config; config->blksize = blocksize; config->bytesize = blocksize * nr_blocks; + if (nbd->task_recv != NULL) + nbd_size_update(nbd); }
static void nbd_complete_rq(struct request *req)
From: Josef Bacik jbacik@fb.com
When we stopped relying on the bdev everywhere I broke updating the block device size on the fly, which ceph relies on. We can't just do set_capacity, we also have to do bd_set_size so things like parted will notice the device size change.
Fixes: 29eaadc ("nbd: stop using the bdev everywhere") cc: stable@vger.kernel.org Signed-off-by: Josef Bacik jbacik@fb.com --- drivers/block/nbd.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 1520383b12f6..61dd95aa47fa 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -231,9 +231,15 @@ static void nbd_size_clear(struct nbd_device *nbd) static void nbd_size_update(struct nbd_device *nbd) { struct nbd_config *config = nbd->config; + struct block_device *bdev = bdget_disk(nbd->disk, 0); + blk_queue_logical_block_size(nbd->disk->queue, config->blksize); blk_queue_physical_block_size(nbd->disk->queue, config->blksize); set_capacity(nbd->disk, config->bytesize >> 9); + if (bdev) { + bd_set_size(bdev, config->bytesize); + bdput(bdev); + } kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE); }
@@ -1111,7 +1117,6 @@ static int nbd_start_device_ioctl(struct nbd_device *nbd, struct block_device *b if (ret) return ret;
- bd_set_size(bdev, config->bytesize); if (max_part) bdev->bd_invalidated = 1; mutex_unlock(&nbd->config_lock);
On Fri, 2018-04-13 at 12:03 -0400, Josef Bacik wrote:
This fixes a use after free bug, we need to do the del_gendisk after we cleanup the queue on the device.
Hello Josef,
Which use-after-free bug does this patch fix? Are you aware that most block drivers call blk_cleanup_queue() before calling del_gendisk()? Why do you think that the nbd driver should use the opposite order?
Thanks,
Bart.
On Fri, Apr 13, 2018 at 04:16:18PM +0000, Bart Van Assche wrote:
On Fri, 2018-04-13 at 12:03 -0400, Josef Bacik wrote:
This fixes a use after free bug, we need to do the del_gendisk after we cleanup the queue on the device.
Hello Josef,
Which use-after-free bug does this patch fix? Are you aware that most block drivers call blk_cleanup_queue() before calling del_gendisk()? Why do you think that the nbd driver should use the opposite order?
I'm confused, that's what this patch does. Before I had del_gendisk() first and then the blk_cleanup_queue(), which was bugging out when I was testing stuff with a null pointer deref whenever I rmmod'ed the nbd. Swapping it to the way everybody else did it fixed the problem. Thanks,
Josef
On Fri, 2018-04-13 at 12:21 -0400, Josef Bacik wrote:
On Fri, Apr 13, 2018 at 04:16:18PM +0000, Bart Van Assche wrote:
On Fri, 2018-04-13 at 12:03 -0400, Josef Bacik wrote:
This fixes a use after free bug, we need to do the del_gendisk after we cleanup the queue on the device.
Hello Josef,
Which use-after-free bug does this patch fix? Are you aware that most block drivers call blk_cleanup_queue() before calling del_gendisk()? Why do you think that the nbd driver should use the opposite order?
I'm confused, that's what this patch does. Before I had del_gendisk() first and then the blk_cleanup_queue(), which was bugging out when I was testing stuff with a null pointer deref whenever I rmmod'ed the nbd. Swapping it to the way everybody else did it fixed the problem. Thanks,
Hello Josef,
Oops, I swapped "blk_cleanup_queue()" and "del_gendisk()" in my e-mail.
Can you share the call stack of the NULL pointer deref and also the translation of the crash address into a source code line?
Thanks,
Bart.
linux-stable-mirror@lists.linaro.org
-
Bart Van Assche -
Josef Bacik