From: "Michael S. Tsirkin" <mst@redhat.com>
To: Yongji Xie <xieyongji@bytedance.com>
Cc: Max Gurtovoy <mgurtovoy@nvidia.com>,
Jason Wang <jasowang@redhat.com>,
Stefan Hajnoczi <stefanha@redhat.com>,
virtualization <virtualization@lists.linux-foundation.org>,
linux-block@vger.kernel.org,
linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v5] virtio-blk: Add validation for block size in config space
Date: Mon, 23 Aug 2021 12:02:58 -0400 [thread overview]
Message-ID: <20210823120229-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <CACycT3sR6Y5XiK6_xX2ni8w9mqmSxkrb639ByDzV2W+Jz79Dnw@mail.gmail.com>
On Mon, Aug 23, 2021 at 08:40:30PM +0800, Yongji Xie wrote:
> On Mon, Aug 23, 2021 at 8:13 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Mon, Aug 23, 2021 at 01:45:31PM +0300, Max Gurtovoy wrote:
> > > It helpful if there is a justification for this.
> > >
> > > In this case, no such HW device exist and the only device that can cause
> > > this trouble today is user space VDUSE device that must be validated by the
> > > emulation VDUSE kernel driver.
> > >
> > > Otherwise, will can create 1000 commit like this in the virtio level (for
> > > example for each feature for each virtio device).
> >
> > Yea, it's a lot of work but I don't think it's avoidable.
> >
> > > >
> > > > > > > > And regardless of userspace device, we still need to fix it for other cases.
> > > > > > > which cases ? Do you know that there is a buggy HW we need to workaround ?
> > > > > > >
> > > > > > No, there isn't now. But this could be a potential attack surface if
> > > > > > the host doesn't trust the device.
> > > > > If the host doesn't trust a device, why it continues using it ?
> > > > >
> > > > IIUC this is the case for the encrypted VMs.
> > >
> > > what do you mean encrypted VM ?
> > >
> > > And how this small patch causes a VM to be 100% encryption supported ?
> > >
> > > > > Do you suggest we do these workarounds in all device drivers in the kernel ?
> > > > >
> > > > Isn't it the driver's job to validate some unreasonable configuration?
> > >
> > > The check should be in different layer.
> > >
> > > Virtio blk driver should not cover on some strange VDUSE stuff.
> >
> > Yes I'm not convinced VDUSE is a valid use-case. I think that for
> > security and robustness it should validate data it gets from userspace
> > right there after reading it.
> > But I think this is useful for the virtio hardening thing.
> > https://lwn.net/Articles/865216/
> >
> > Yongji - I think the commit log should be much more explicit that
> > this is hardening. Otherwise people get confused and think this
> > needs a CVE or a backport for security.
> >
>
> OK, do I need to send a v6? This patch seems to be already merged into
> Linus's tree.
>
> Thanks,
> Yongji
No, it's a comment for the future - I assume you will keep adding this
kind of validation in other places.
--
MST
next prev parent reply other threads:[~2021-08-23 16:03 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-09 10:16 [PATCH v5] virtio-blk: Add validation for block size in config space Xie Yongji
2021-08-10 3:05 ` Jason Wang
2021-08-10 4:59 ` Yongji Xie
2021-08-10 6:59 ` Jason Wang
2021-08-22 23:17 ` Max Gurtovoy
2021-08-23 4:31 ` Yongji Xie
2021-08-23 8:07 ` Max Gurtovoy
2021-08-23 8:35 ` Yongji Xie
2021-08-23 9:04 ` Max Gurtovoy
2021-08-23 9:27 ` Yongji Xie
2021-08-23 9:38 ` Max Gurtovoy
2021-08-23 10:33 ` Yongji Xie
2021-08-23 10:45 ` Max Gurtovoy
2021-08-23 11:41 ` Yongji Xie
2021-08-23 12:13 ` Michael S. Tsirkin
2021-08-23 12:40 ` Yongji Xie
2021-08-23 16:02 ` Michael S. Tsirkin [this message]
2021-08-23 22:31 ` Max Gurtovoy
2021-08-24 2:47 ` Jason Wang
2021-08-24 10:11 ` Max Gurtovoy
2021-08-24 12:52 ` Yongji Xie
2021-08-24 13:30 ` Max Gurtovoy
2021-08-24 13:38 ` Yongji Xie
2021-08-24 13:48 ` Max Gurtovoy
2021-10-04 15:27 ` Michael S. Tsirkin
2021-10-04 15:39 ` Michael S. Tsirkin
2021-10-05 15:52 ` Yongji Xie
2021-10-05 10:42 ` Michael S. Tsirkin
2021-10-05 15:45 ` Yongji Xie
2021-10-05 18:26 ` Martin K. Petersen
2021-10-11 11:40 ` Christoph Hellwig
2021-10-13 12:21 ` Michael S. Tsirkin
2021-10-13 12:34 ` Yongji Xie
2021-10-13 12:51 ` Michael S. Tsirkin
2021-10-13 12:59 ` Yongji Xie
2021-10-05 15:24 ` Yongji Xie
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210823120229-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=jasowang@redhat.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mgurtovoy@nvidia.com \
--cc=stefanha@redhat.com \
--cc=virtualization@lists.linux-foundation.org \
--cc=xieyongji@bytedance.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).