On 23/04/2024 3:44 pm, Jan Beulich wrote:
> On 22.04.2024 20:14, Andrew Cooper wrote:
>> In debug builds, walk all alternative replacements with x86_decode_lite().
>>
>> This checks that we can decode all instructions, and also lets us check that
>> disp8's don't leave the replacement block.
>>
>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
> With pointed-to types consistently constified, technically:
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
Thanks.
>> @@ -464,6 +465,54 @@ static void __init _alternative_instructions(bool force)
>> void __init alternative_instructions(void)
>> {
>> arch_init_ideal_nops();
>> +
>> + /*
>> + * Walk all replacement instructions with x86_decode_lite(). This
>> checks
>> + * both that we can decode all instructions within the replacement, and
>> + * that any near branch with a disp8 stays within the alternative
>> itself.
>> + */
>> + if ( IS_ENABLED(CONFIG_DEBUG) )
I've swapped this for SELF_TESTS now it exists.
>> + }
>> +
>> + if ( res.rel_type == REL_TYPE_d8 )
>> + {
>> + int8_t *d8 = res.rel;
>> + void *target = ip + res.len + *d8;
>> +
>> + if ( target < repl || target > end )
>> + {
>> + printk("Alternative for %ps [%*ph]\n",
>> + ALT_ORIG_PTR(a), a->repl_len, repl);
>> + panic("'JMP/Jcc disp8' at +%u leaves alternative
>> block\n",
>> + (unsigned int)(unsigned long)(ip - repl));
>> + }
>> + }
> Why's Disp8 more important to check than Disp32? A bad CALL in a
> replacement can't possibly be encoded with Disp8, and both JMP and Jcc
> are also more likely to be encoded with Disp32 when their target isn't
> in the same blob (but e.g. in a different section).
Whatever the likelihood of them existing, Disp8's cannot possibly be
correct if they cross the boundary of the replacement. Checking for
them has the side effect of running decode_lite() over all replacements.
Disp32's do exist in both external and internal forms (retpoline), and
the point of this series is to make all external cases usable.
Therefore, there are no invalid cases.
~Andrew
