Re: [PATCH v5] sev: add sev-inject-launch-secret

[Tobin Feldman-Fitzthum]

On 2020-10-20 11:56, Paolo Bonzini wrote:
> On 20/10/20 15:54, Eduardo Habkost wrote:
> > On Tue, Oct 20, 2020 at 11:03:51AM +0200, Paolo Bonzini wrote:
> > > On 15/10/20 16:37, tobin@linux.ibm.com wrote:
> > > > -static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error 
> > > > **errp)
> > > > +void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, 
> > > > Error **errp)
> > > >  {
> > > >      MemoryRegionSection mrs = 
> > > > memory_region_find(get_system_memory(),
> > > > -                                                 addr, 1);
> > > > +                                                 addr, size);
> > >
> > > You need to check size against mrs.size and fail if mrs.size is 
> > > smaller.
> > >  Otherwise, the ioctl can access memory out of range.
> >
> > Good catch!  I'm dequeuing it.
> >
> > Is there a reason memory_region_find() doesn't ensure that by
> > default?
>
> IIRC memory_region_find() was used to do DMA in the very first versions
> of "virtio-blk dataplane" so you would call it multiple times in a 
> loop.
>  So it's like that because it maps the way address_space_map() works.
>
> > The call at virtio_balloon_handle_output() looks suspicious,
> > though, because it looks for a BALLOON_PAGE_SIZE range, but
> > there's no check for the returned section size.
>
> I think it's not a bug because ultimately it's checked in
> ram_block_discard_range, but it's not pretty.
>
> Paolo

Ok, it seems like the best solution is, as Paolo suggested, to add a
simple check at the end of gpa2hva that makes sure mr.size is equal
to size. gpa2hva is used one other place where the size is hard-coded
as 1, so adding the check isn't going to break anything.

Would you like me to resubmit with this tweak?

-Tobin