On Tue, Aug 24, 2021 at 9:30 PM Max Gurtovoy <mgurtovoy@xxxxxxxxxx> wrote: > > > On 8/24/2021 3:52 PM, Yongji Xie wrote: > > On Tue, Aug 24, 2021 at 6:11 PM Max Gurtovoy <mgurtovoy@xxxxxxxxxx> wrote: > >> > >> On 8/24/2021 5:47 AM, Jason Wang wrote: > >>> On Tue, Aug 24, 2021 at 6:31 AM Max Gurtovoy <mgurtovoy@xxxxxxxxxx> wrote: > >>>> On 8/23/2021 3:13 PM, Michael S. Tsirkin 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/ > >>>> I don't see how this change is assisting confidential computing. > >>>> > >>>> Confidential computingtalks about encrypting guest memory from the host, > >>>> and not adding some quirks to devices. > >>> In the case of confidential computing, the hypervisor and hard device > >>> is not in the trust zone. It means the guest doesn't trust the cloud > >>> vendor. > >> Confidential computing protects data during processing ("in-use" data). > >> > >> Nothing to do with virtio feature negotiation. > >> > > But if a misbehaving device can corrupt the guest memory, I think it > > should be avoided. > > So don't say it's related to confidential computing, and fix it in the > VDUSE kernel driver in the hypervisor. > What I mean is in confidential computing cases. An untrusted device might corrupt the protected guest memory, it should be avoided. Thanks, Yongji