Virtualization - Jan 2023 - [PATCH v1 2/6] virtio console: Harden port adding

On Fri, Jan 27, 2023 at 02:47:55PM +0200, Alexander Shishkin
wrote:
> "Michael S. Tsirkin" <mst at redhat.com> writes:
> 
> > On Fri, Jan 27, 2023 at 01:55:43PM +0200, Alexander Shishkin wrote:
> >> "Michael S. Tsirkin" <mst at redhat.com> writes:
> >> 
> >> > On Thu, Jan 19, 2023 at 10:13:18PM +0200, Alexander Shishkin
wrote:
> >> >> When handling control messages, instead of peeking at the
device memory
> >> >> to obtain bits of the control structure,
> >> >
> >> > Except the message makes it seem that we are getting data
from
> >> > device memory, when we do nothing of the kind.
> >> 
> >> We can be, see below.
> >> 
> >> >> take a snapshot of it once and
> >> >> use it instead, to prevent it from changing under us.
This avoids races
> >> >> between port id validation and control event decoding,
which can lead
> >> >> to, for example, a NULL dereference in port removal of a
nonexistent
> >> >> port.
> >> >> 
> >> >> The control structure is small enough (8 bytes) that it
can be cached
> >> >> directly on the stack.
> >> >
> >> > I still have no real idea why we want a copy here.
> >> > If device can poke anywhere at memory then it can crash
kernel anyway.
> >> > If there's a bounce buffer or an iommu or some other
protection
> >> > in place, then this memory can no longer change by the time
> >> > we look at it.
> >> 
> >> We can have shared pages between the host and guest without bounce
> >> buffers in between, so they can be both looking directly at the
same
> >> page.
> >> 
> >> Regards,
> >
> > How does this configuration work? What else is in this page?
> 
> So, for example in TDX, you have certain pages as "shared", as in
> between guest and hypervisor. You can have virtio ring(s) in such
> pages. It's likely that there'd be a swiotlb buffer there instead,
but
> sharing pages between host virtio and guest virtio drivers is possible.
If it is shared, then what does this mean?  Do we then need to copy
everything out of that buffer first before doing anything with it
because the data could change later on?  Or do we not trust anything in
it at all and we throw it away?  Or something else (trust for a short
while and then we don't?)

Please be specific as to what you want to see happen here, and why.

thanks,

greg k-h