[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH L1TF v8 9/9] common/grant_table: block speculative out-of-bound accesses



>>> On 27.02.19 at 14:01, <nmanthey@xxxxxxxxx> wrote:
> On 2/25/19 17:46, Jan Beulich wrote:
>> I would really like to ask that I (or someone else) don't need to
>> go through and list remaining version checks again - after all I
>> had done so for v6 already, and I didn't go through all of them
>> again for v7 assuming that you would have worked through the
>> entire set.
> 
> So, here is the annotation for all of them. Anyone that I did not
> include in the list has been fixed in previous versions, or will be
> fixed in the next version:
> 
> git grep -np version common/grant_table.c
> 
> common/grant_table.c:831:static int _set_status(unsigned gt_version,
> common/grant_table.c:840:    if ( gt_version == 1 )
> 
> -> I do not see how out-of-bound accesses happen in the called functions
> there.

Both functions get shah passed into them, which may point to the
other version's layout. Earlier fences don't help speculation on this
conditional.

> common/grant_table.c=1444=unmap_common_complete(struct
> gnttab_unmap_common *op)
> common/grant_table.c:1469:    if ( rgt->gt_version == 1 )
> 
> -> I do not see how to be exploitable, as the shared_entry_header call
> above just used an lfence.

Again, earlier fences don't suppress subsequent speculation on
an independent conditional.

> common/grant_table.c=1761=gnttab_grow_table(struct domain *d, unsigned
> int req_nr_frames)
> common/grant_table.c:1800:    /* Status pages - version 2 */
> common/grant_table.c:1801:    if ( gt->gt_version > 1 )
> 
> -> I do not see how out-of-bound access could happen. This calls
> gnttab_populate_status_frames that allocates pages and should not touch
> more memory than before

We've been talking about the speculation window being perhaps
hundreds of insns. It may be purely theoretical, but speculation all
the way through alloc_xenheap_page() would lead to a speculative
store to gt->status[]. _Right now_ that array gets allocated in
grant_table_init(), but I can't see why that couldn't be moved to
gnttab_set_version(), so the access above could be a latent issue.

I'll skip going into details of further ones, assuming that some of
them follow patterns above.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.