Re: [PATCH 1/1] virtio: disable partitions scanning for no partitions block

From: Stefan Hajnoczi <stefanha@redhat.com>
To: Yury Kamenev <damtev@yandex-team.ru>
Cc: mst@redhat.com, jasowang@redhat.com, pbonzini@redhat.com,
	axboe@kernel.dk, virtualization@lists.linux-foundation.org,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH 1/1] virtio: disable partitions scanning for no partitions block
Date: Mon, 24 May 2021 15:29:22 +0100	[thread overview]
Message-ID: <YKu4Qovv1KMplifY@stefanha-x1.localdomain> (raw)
In-Reply-To: <20210520133908.98891-2-damtev@yandex-team.ru>

[-- Attachment #1: Type: text/plain, Size: 3246 bytes --]

On Thu, May 20, 2021 at 04:39:08PM +0300, Yury Kamenev wrote:

Hi,
Is there a VIRTIO spec change for the new VIRTIO_BLK_F_NO_PS feature
bit? Please send one:
https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=virtio#feedback

GENHD_FL_NO_PART_SCAN is not used much in other drivers. This makes me
wonder if the same use case is addressed through other means with SCSI,
NVMe, etc devices. Maybe Christoph or Jens can weigh in on whether
adding a bit to disable partition scanning for a virtio-blk fits into
the big picture?

Is your goal to avoid accidentally detecting partitions because it's
confusing when that happens?

VIRTIO is currently undergoing auditing and changes to support untrusted
devices. From that perspective adding a device feature bit to disable
partition scanning does not help protect the guest from an untrusted
disk. The guest cannot trust the device, instead the guest itself would
need to be configured to avoid partition scanning of untrusted devices.

Stefan

> Signed-off-by: Yury Kamenev <damtev@yandex-team.ru>
> ---
>  drivers/block/virtio_blk.c      | 6 ++++++
>  include/uapi/linux/virtio_blk.h | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index b9fa3ef5b57c..17edcfee2208 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -799,6 +799,10 @@ static int virtblk_probe(struct virtio_device *vdev)
>  	vblk->disk->flags |= GENHD_FL_EXT_DEVT;
>  	vblk->index = index;
>  
> +	/*Disable partitions scanning for no-partitions block*/

Formatting cleanup and rephrasing:

  /* Disable partition scanning for devices with no partitions */

> +	if (virtio_has_feature(vdev, VIRTIO_BLK_F_NO_PS))

I suggest user a more obvious name:

  VIRTIO_BLK_F_NO_PART_SCAN

> +		vblk->disk->flags |= GENHD_FL_NO_PART_SCAN;
> +
>  	/* configure queue flush support */
>  	virtblk_update_cache_mode(vdev);
>  
> @@ -977,6 +981,7 @@ static unsigned int features_legacy[] = {
>  	VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
>  	VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
>  	VIRTIO_BLK_F_MQ, VIRTIO_BLK_F_DISCARD, VIRTIO_BLK_F_WRITE_ZEROES,
> +	VIRTIO_BLK_F_NO_PS,
>  }
>  ;
>  static unsigned int features[] = {
> @@ -984,6 +989,7 @@ static unsigned int features[] = {
>  	VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
>  	VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
>  	VIRTIO_BLK_F_MQ, VIRTIO_BLK_F_DISCARD, VIRTIO_BLK_F_WRITE_ZEROES,
> +	VIRTIO_BLK_F_NO_PS,
>  };
>  
>  static struct virtio_driver virtio_blk = {
> diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
> index d888f013d9ff..f197d07afb05 100644
> --- a/include/uapi/linux/virtio_blk.h
> +++ b/include/uapi/linux/virtio_blk.h
> @@ -40,6 +40,7 @@
>  #define VIRTIO_BLK_F_MQ		12	/* support more than one vq */
>  #define VIRTIO_BLK_F_DISCARD	13	/* DISCARD is supported */
>  #define VIRTIO_BLK_F_WRITE_ZEROES	14	/* WRITE ZEROES is supported */
> +#define VIRTIO_BLK_F_NO_PS      16      /* No partitions */
>  
>  /* Legacy feature bits */
>  #ifndef VIRTIO_BLK_NO_LEGACY
> -- 
> 2.24.3 (Apple Git-128)
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]