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

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-next>] [day] [month] [year] [list]
Message-ID: <21b08ea7-71c6-1b4b-3833-1c51d0e1d310@redhat.com>
Date:   Mon, 24 May 2021 21:41:01 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Юрий Каменев 
        <damtev@...dex-team.ru>, Stefan Hajnoczi <stefanha@...hat.com>
Cc:     "mst@...hat.com" <mst@...hat.com>,
        "jasowang@...hat.com" <jasowang@...hat.com>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Christoph Hellwig <hch@....de>
Subject: Re: [PATCH 1/1] virtio: disable partitions scanning for no partitions
 block

On 24/05/21 21:34, Юрий Каменев wrote:
> Hi
> 
>     Is your goal to avoid accidentally detecting partitions because it's
>     confusing when that happens?
> 
> The main goal is reducing the kernel start time. It might be use useful 
> in tiny systems that use, for example, squashfs images with certainly no 
> partitions. Disabling partitions scanning for these images can save a 
> few tens of milliseconds which can be a significant acceleration for 
> starting such systems.

Perhaps that could be configured in the image, for example in the kernel 
command line?

Paolo

> 24.05.2021, 17:29, "Stefan Hajnoczi" <stefanha@...hat.com>:
> 
>     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
>     <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@...dex-team.ru
>         <mailto:damtev@...dex-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)
>