Re: [Xen-devel] [PATCH SpectreV1+L1TF v5 9/9] common/memory: block speculative out-of-bound accesses

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.01.19 at 15:43, <nmanthey@xxxxxxxxx> wrote:
> @@ -33,10 +34,10 @@ unsigned long __read_mostly pdx_group_valid[BITS_TO_LONGS(
>  
>  bool __mfn_valid(unsigned long mfn)
>  {
> -    return likely(mfn < max_page) &&
> -           likely(!(mfn & pfn_hole_mask)) &&
> -           likely(test_bit(pfn_to_pdx(mfn) / PDX_GROUP_COUNT,
> -                           pdx_group_valid));
> +    return evaluate_nospec(likely(mfn < max_page) &&
> +                           likely(!(mfn & pfn_hole_mask)) &&
> +                           likely(test_bit(pfn_to_pdx(mfn) / PDX_GROUP_COUNT,
> +                                           pdx_group_valid)));

Other than in the questionable grant table case, here I agree that
you want to wrap the entire construct. This has an unwanted effect
though: The test_bit() may still be speculated into with an out-of-
bounds mfn. (As mentioned elsewhere, operations on bit arrays are
an open issue altogether.) I therefore think you want to split this into
two:

bool __mfn_valid(unsigned long mfn)
{
    return likely(evaluate_nospec(mfn < max_page)) &&
           evaluate_nospec(likely(!(mfn & pfn_hole_mask)) &&
                           likely(test_bit(pfn_to_pdx(mfn) / PDX_GROUP_COUNT,
                                           pdx_group_valid)));
}

Jan



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