Skip to content

Morello: disable VMAP_STACK - #24

Open
heshamelmatary wants to merge 1 commit into
codasip-cheri-riscv-7.0from
vmap_stack
Open

Morello: disable VMAP_STACK#24
heshamelmatary wants to merge 1 commit into
codasip-cheri-riscv-7.0from
vmap_stack

Conversation

@heshamelmatary

Copy link
Copy Markdown
Collaborator

VMAP_STACK checks for kernel stack overflows on kernel entry and reports a fault if the checks fail. This was originally disabled when the Morello port was developed. Since then, commit ef6861b (see [1]) enabled VMAP_STACK by default on arm64.

Unfortunately, the VMAP_STACK entry code clobbers [c]sp on entry, causing the stack pointer capability to lose its tag. Since there are not enough scratch registers available at that stage of execution, this commit explicitly disables VMAP_STACK again for Morello.

[1] https://lore.kernel.org/all/20250707-arm64_vmap-v1-0-8de98ca0f91c@debian.org/

VMAP_STACK checks for kernel stack overflows on kernel entry
and reports a fault if the checks fail. This was originally
disabled when the Morello port was developed. Since then,
commit ef6861b (see [1]) enabled VMAP_STACK by default on
arm64.

Unfortunately, the VMAP_STACK entry code clobbers [c]sp
on entry, causing the stack pointer capability to lose its
tag. Since there are not enough scratch registers available
at that stage of execution, this commit explicitly disables
VMAP_STACK again for Morello.

[1] https://lore.kernel.org/all/20250707-arm64_vmap-v1-0-8de98ca0f91c@debian.org/

Signed-off-by: Hesham Almatary <hesham.almatary@cl.cam.ac.uk>
@kevin-brodsky-arm

Copy link
Copy Markdown
Collaborator

I don't think this is sufficient. The patch that enabled VMAP_STACK conditionally is actually part of a series of 8 patches, quite a few things behaved differently without VMAP_STACK: https://lore.kernel.org/all/20250707-arm64_vmap-v1-0-8de98ca0f91c@debian.org/

@kevin-brodsky-arm

Copy link
Copy Markdown
Collaborator

Reverting the whole series isn't practical IMHO, things are likely to go wrong when rebasing. I think there are two options:

  1. The minimal hack that I had in mind was to simply not do the stack overflow check at the beginning of kernel_ventry. This should make no difference under normal conditions. If an overflow does happen, then rather than a nice panic it will go horribly wrong and most likely get stuck in a recursive exception. Not great, but definitely good enough to experiment.
  2. I think there is hope that we can stash CSP somewhere else to keep doing this stack overflow check. Most likely candidate is CTPIDR_EL1 (or CTPIDR_EL2, see set_my_cpu_offset()). It is used by the kernel but its value never changes, so I'm pretty sure we could overwrite it on exception entry and then restore it after the stack overflow check.

@heshamelmatary

Copy link
Copy Markdown
Collaborator Author

Reverting the whole series isn't practical IMHO, things are likely to go wrong when rebasing. I think there are two options:

  1. The minimal hack that I had in mind was to simply not do the stack overflow check at the beginning of kernel_ventry.

This wasn't enough unfortunately. There was another bad stack check during an exit system call and a task clean-up in the kernel's exit_task_stack_account that was triggering another fault/panic if CONFIG_VMAP_STACK is enabled.

This should make no difference under normal conditions. If an overflow does happen, then rather than a nice panic it will go horribly wrong and most likely get stuck in a recursive exception. Not great, but definitely good enough to experiment.
2. I think there is hope that we can stash CSP somewhere else to keep doing this stack overflow check. Most likely candidate is CTPIDR_EL1 (or CTPIDR_EL2, see set_my_cpu_offset()). It is used by the kernel but its value never changes, so I'm pretty sure we could overwrite it on exception entry and then restore it after the stack overflow check.

I can have a look at implementing that later. I'd need to prioritise the purecap Morello kernel though as hybrid will most likely be obsolete. I was just trying to do the bare minimum for hybrid to get a shell.

@kevin-brodsky-arm

Copy link
Copy Markdown
Collaborator

Ah right then you'd need to disable all overflow checks. Unfortunately it looks like this is going to be quite a bit of work to maintain no matter what you do.

I don't think there should be much difference between hybrid and purecap kernel if you were to amend the checks to use e.g. CTPIDR_EL1.

chrehrhardt pushed a commit that referenced this pull request Jun 18, 2026
The xfstests' test-case generic/523 fails to execute
correctly:

FSTYP -- hfsplus
PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.15.0-rc4+ #8 SMP PREEMPT_DYNAMIC Thu May 1 16:43:22 PDT 2025
MKFS_OPTIONS -- /dev/loop51
MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch

generic/523 - output mismatch (see xfstests-dev/results//generic/523.out.bad)

The test-case expects to have '/' in the xattr name.
However, HFS+ unicode logic makes conversion of '/'
into ':'. In HFS+, a filename can contain '/' because
':' is the separator. The slash is a valid filename
character on macOS. But on Linux, / is the path separator
and it cannot appear in a filename component. But xattr
name can contain any of these symbols. It means that
this unicode logic conversion doesn't need to be executed
for the case of xattr name.

This patch adds distinguishing the regular and xattr names.
If we have a regular name, then this conversion of special
symbols will be executed. Otherwise, the conversion is skipped
for the case of xattr names.

sudo ./check -g auto
FSTYP         -- hfsplus
PLATFORM      -- Linux/x86_64 hfsplus-testing-0001 7.0.0-rc1+ #24 SMP PREEMPT_DYNAMIC Fri Mar 20 12:36:49 PDT 2026
MKFS_OPTIONS  -- /dev/loop51
MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch

<skipped>
generic/523 33s ...  25s
<skipped>

Closes: hfs-linux-kernel#178
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
Link: https://lore.kernel.org/r/20260324003949.417048-2-slava@dubeyko.com
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants