On 27/03/2023 4:43 pm, Jan Beulich wrote:
> On 24.03.2023 23:08, Andrew Cooper wrote:
>> While we've been diligent to ensure that the main text/data/rodata mappings
>> have suitable restrictions, their aliases via the directmap were left fully
>> read/write. Worse, we even had pieces of code making use of this as a
>> feature.
>>
>> Restrict the permissions for .text/rodata, as we have no legitimate need for
>> writeability of these areas via the directmap alias. Note that the
>> compile-time allocated pagetables do get written through their directmap
>> alias, so need to remain writeable.
>>
>> Signed-off-by: Andrew Cooper <[email protected]>
> Reviewed-by: Jan Beulich <[email protected]>
Thanks.
>
>> Notes:
>> * The stubs are still have RX via one alias, RW via another, and these need
>> to stay. We should harden this using PKS (available on SPR and later) to
>> block incidental writes.
>> * Backing memory for livepatch text/rodata needs similar treatment.
> Right, but there it's somewhat more involved because upon removal the
> attributes also need restoring.
Yeah, I'm going to defer it to a gitlab ticket for now. And the PKS
addition too.
>> * For backporting, this patch depends on c/s e7f147bf4ac7 ("x86/crash: Drop
>> manual hooking of exception_table[]") and c/s e7db635f4428 ("x86/pv-shim:
>> Don't modify the hypercall table"). No compile error will occur from
>> getting these dependencies wrong.
> I suppose the latter isn't strictly a prereq, as the modification was done
> from an __init function (i.e. before this new code runs).
This code here runs long before pv_shim_setup_dom(). This dependency
was found experimentally via testing IIRC.
> Iirc we didn't backport prior similar hardening work? So I'm not sure we'd
> want/need to do so in this case.
That patch wasn't backported. I was originally planning to, as part of
the CET-IBT work for Intel-retbleed, but in the end didn't.
We went for the "ENDBR on every function" approach on older trees
because it was better than nothing, and far less invasive than
backporting cf_check everywhere.
~Andrew
