On Tue, Feb 21, 2023 at 09:34:35AM +0100, David Hildenbrand wrote:
On 20.02.23 23:08, Edgecombe, Rick P wrote:
On Mon, 2023-02-20 at 13:56 +0100, David Hildenbrand wrote:
On 18.02.23 22:14, Rick Edgecombe wrote:
From: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx>
The x86 Control-flow Enforcement Technology (CET) feature includes
a new
type of memory called shadow stack. This shadow stack memory has
some
unusual properties, which requires some core mm changes to function
properly.
A shadow stack PTE must be read-only and have _PAGE_DIRTY set.
However,
read-only and Dirty PTEs also exist for copy-on-write (COW) pages.
These
two cases are handled differently for page faults. Introduce
VM_SHADOW_STACK to track shadow stack VMAs.
I suggest simplifying and abstracting that description.
"New hardware extensions implement support for shadow stack memory,
such
as x86 Control-flow Enforcement Technology (CET). Let's add a new VM
flag to identify these areas, for example, to be used to properly
indicate shadow stack PTEs to the hardware."
Ah yea, that top blurb was added to all the non-x86 arch patches after
some feedback from Andrew Morton. He had said basically (in some more
colorful language) that the changelogs (at the time) were written
assuming the reader knows what a shadow stack is.
Okay. It's a bit repetitive, though.
Ideally, we'd just explain it in the cover letter in detail and
Andrews's script would include the cover letter in the first commit.
IIRC, that's what usually happens.
So it might be worth keeping a little more info in the log?
Copying the same paragraph into each commit is IMHO a bit repetitive.
But these are just my 2 cents.
[...]
Should we abstract this to CONFIG_ARCH_USER_SHADOW_STACK, seeing
that
other architectures might similarly need it?
There was an ARCH_HAS_SHADOW_STACK but it got removed following this
discussion:
https://lore.kernel.org/lkml/d09e952d8ae696f687f0787dfeb7be7699c02913.camel@xxxxxxxxx/
Now we have this new RFC for riscv as potentially a second
implementation. But it is still very early, and I'm not sure anyone
knows exactly what the similarities will be in a mature version. So I
think it would be better to refactor in an ARCH_HAS_SHADOW_STACK later
(and similar abstractions) once that series is more mature and we have
an idea of what pieces will be shared. I don't have a problem in
principle with an ARCH config, just don't think we should do it yet.
Okay, easy to factor out later.
I would be more than happy if this config name would've been abstracted out and arches can
choose to implement. It's a bit sad that it was generic earlier and was later changed due
to lack of support from other architectures. Now there are three architectures who either
already support shadow stack (x86), announced the support (aarch64) or are planning to
support (riscv).
However given patch reduction I will get due to `pte_mkwrite` refactor, I am in favor of
future refactor for config.
Acked-by: David Hildenbrand <david@xxxxxxxxxx>
--
Thanks,
David / dhildenb
