Re: [Xen-devel] [PATCH v2 1/2] x86/xpti: really hide almost all of Xen image

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/03/18 14:34, Jan Beulich wrote:
> Commit 422588e885 ("x86/xpti: Hide almost all of .text and all
> .data/.rodata/.bss mappings") carefully limited the Xen image cloning to
> just entry code, but then overwrote the just allocated and populated L3
> entry with the normal one again covering both Xen image and stubs.

Some version of this patch definitely worked correctly, but it is clear
that this version doesn't.  Sorry :(

>
> Drop the respective code in favor of an explicit clone_mapping()
> invocation. This in turn now requires setup_cpu_root_pgt() to run after
> stub setup in all cases. Additionally, with (almost) no unintended
> mappings left, the BSP's IDT now also needs to be page aligned.
>
> Note that the removed BUILD_BUG_ON()s don't get replaced by anything -
> there already is a suitable ASSERT() in xen.lds.S.

This isn't quite true.  You've changed the mechanism by which the stubs
get mapped (from entirely common, to per-pcpu), removing the need for
the BUILD_BUG_ON().

The ASSERT() in xen.lds.S serves a different purpose, checking that the
sum total of stubs don't overlap with the compiled code.  (On this
note... do we perform the same check for livepatches?  I can't spot
anything.)

>
> The moving ahead of cleanup_cpu_root_pgt() is not strictly necessary
> for functionality, but things are more logical this way, and we retain
> cleanup being done in the inverse order of setup.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v2: Add missing cleanup of the stub mapping.
> ---
> What should we do with the TSS? Currently together with it we expose
> almost a full page of other per-CPU data. A simple (but slightly
> hackish) option would be to use one of the two unused stack slots.

In 64bit, the TSS can be mapped read-only, because hardware never has
cause to write to it.

I believe that Linux now uses a read-only TSS mapping to double as a
guard page for the trampoline stack, which is a less hacky way of
thinking about it.

However, doing that in Xen would mean shattering the directmap
superpages in all cases, and we'd inherit the SVM triple fault case into
release builds.  A different alternative (and perhaps simpler to
backport) might be to have .bss.percpu.page_aligned and use that to hide
the surrounding data.

Thinking about it, we've got the same problem with the TSS as the BSP
IDT, if the link order happens to cause init_tss to cross a page boundary.

~Andrew

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