linux-kernel - Re: [PATCH] vdpa_sim_blk: add support for VIRTIO_BLK_T

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220429071449.pycbkk2dvvxmtvay@sgarzare-redhat>
Date:   Fri, 29 Apr 2022 09:14:49 +0200
From:   Stefano Garzarella <sgarzare@...hat.com>
To:     Jason Wang <jasowang@...hat.com>
Cc:     virtualization <virtualization@...ts.linux-foundation.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Stefan Hajnoczi <stefanha@...hat.com>,
        Max Gurtovoy <mgurtovoy@...dia.com>
Subject: Re: [PATCH] vdpa_sim_blk: add support for VIRTIO_BLK_T_FLUSH

On Fri, Apr 29, 2022 at 10:46:40AM +0800, Jason Wang wrote:
>On Thu, Apr 28, 2022 at 11:13 PM Stefano Garzarella <sgarzare@...hat.com> wrote:
>>
>> The simulator behaves like a ramdisk, so we don't have to do
>> anything when a VIRTIO_BLK_T_FLUSH request is received, but it
>> could be useful to test driver behavior.
>>
>> Let's expose the VIRTIO_BLK_F_FLUSH feature to inform the driver
>> that we support the flush command.
>>
>> Signed-off-by: Stefano Garzarella <sgarzare@...hat.com>
>> ---
>>  drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
>> index 42d401d43911..a6dd1233797c 100644
>> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
>> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
>> @@ -25,6 +25,7 @@
>>  #define DRV_LICENSE  "GPL v2"
>>
>>  #define VDPASIM_BLK_FEATURES   (VDPASIM_FEATURES | \
>> +                                (1ULL << VIRTIO_BLK_F_FLUSH)    | \
>>                                  (1ULL << VIRTIO_BLK_F_SIZE_MAX) | \
>>                                  (1ULL << VIRTIO_BLK_F_SEG_MAX)  | \
>>                                  (1ULL << VIRTIO_BLK_F_BLK_SIZE) | \
>> @@ -166,6 +167,17 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim,
>>                 pushed += bytes;
>>                 break;
>>
>> +       case VIRTIO_BLK_T_FLUSH:
>> +               if (sector != 0) {
>> +                       dev_err(&vdpasim->vdpa.dev,
>> +                               "A driver MUST set sector to 0 for a VIRTIO_BLK_T_FLUSH request - sector: 0x%llx\n",
>> +                               sector);
>
>If this is something that could be triggered by userspace/guest, then
>we should avoid this.

It can only be triggered by an erratic driver.

I was using the simulator to test a virtio-blk driver that I'm writing 
in userspace and I forgot to set `sector` to zero, so I thought it would 
be useful.

Do you mean to remove the error message?

Thanks,
Stefano