[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH SpectreV1+L1TF v5 8/9] common/grant_table: block speculative out-of-bound accesses
>>> On 29.01.19 at 15:43, <nmanthey@xxxxxxxxx> wrote: > @@ -963,6 +965,9 @@ map_grant_ref( > PIN_FAIL(unlock_out, GNTST_bad_gntref, "Bad ref %#x for d%d\n", > op->ref, rgt->domain->domain_id); > > + /* Make sure the above check is not bypassed speculatively */ > + op->ref = array_index_nospec(op->ref, nr_grant_entries(rgt)); > + > act = active_entry_acquire(rgt, op->ref); > shah = shared_entry_header(rgt, op->ref); > status = rgt->gt_version == 1 ? &shah->flags : &status_entry(rgt, > op->ref); Just FTR - this is a case where the change, according to prior discussion, is pretty unlikely to help at all. The compiler will have a hard time realizing that it could keep the result in a register past the active_entry_acquire() invocation, as that - due to the spin lock acquired there - acts as a compiler barrier. And looking at generated code (gcc 8.2) confirms that there's a reload from the stack. > @@ -2026,6 +2031,9 @@ gnttab_prepare_for_transfer( > goto fail; > } > > + /* Make sure the above check is not bypassed speculatively */ > + ref = array_index_nospec(ref, nr_grant_entries(rgt)); > + > sha = shared_entry_header(rgt, ref); > > scombo.word = *(u32 *)&sha->flags; > @@ -2223,7 +2231,8 @@ gnttab_transfer( > okay = gnttab_prepare_for_transfer(e, d, gop.ref); > spin_lock(&e->page_alloc_lock); > > - if ( unlikely(!okay) || unlikely(e->is_dying) ) > + /* Make sure this check is not bypassed speculatively */ > + if ( evaluate_nospec(unlikely(!okay) || unlikely(e->is_dying)) ) I'm still not really happy about this. The comment isn't helpful in connecting the use of evaluate_nospec() to the problem site (in the earlier hunk, which I've left in context), and I still don't understand why the e->is_dying is getting wrapped as well. Plus it occurs to me now that you're liable to render unlikely() ineffective here. So how about if ( unlikely(evaluate_nospec(!okay)) || unlikely(e->is_dying) ) ? Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |