On Thu, Apr 03, 2025 at 08:20:23AM +0200, Benjamin Berg wrote:
> Hi Nathan,
>
> oops, that is a little logic bug in the code. When we are coming from
> userspace, we may be running as part of a user task and have an mm. And
> then the new "current->pagefault_disabled" logic is skipped. So when
> prepend_copy is doing its nofault copy to evade a lock it is crashing
> instead of failing gracefully and retrying.
>
> In segv_handler, can you try moving that into a separate "else if"
> block just above the "current->mm == NULL" check?
>
> Something like the patch I copied below.
Thanks, I applied that change, which shows a slightly different crash
message now:
Modules linked in:
Pid: 24, comm: mount Not tainted 6.14.0-rc5-00002-gd1d7f01f7cd3-dirty
RIP: 0033:_end_0+0x38/0x45
RSP: 00000000648bfca0 EFLAGS: 00010206
RAX: 0000000000000000 RBX: 00000000609c7ff8 RCX: 00000000606350b8
RDX: 00000000648bfc5e RSI: 0000000000001000 RDI: 0000000060c0c000
RBP: 00000000648bfcc0 R08: 0000000000000000 R09: 0000000060c0c780
R10: 0000000000000000 R11: 0000000000000206 R12: 00000000648bfd60
R13: 0000000060c0c900 R14: 0000000060c0c9f8 R15: 0000000000000007
Kernel panic - not syncing: Segfault without recovery target
CPU: 0 UID: 0 PID: 24 Comm: mount Not tainted
6.14.0-rc5-00002-gd1d7f01f7cd3-dirty #1
Stack:
600c25f9 00000007 609c7ff8 608c4780
648bfcf0 601533da 601533bb 648bfd60
648bfdb0 608c4780 648bfd10 60152ece
Call Trace:
[<600c25f9>] ? copy_from_kernel_nofault+0x0/0x64
[<601533da>] prepend_copy+0x1f/0x51
[<601533bb>] ? prepend_copy+0x0/0x51
[<60152ece>] prepend+0x60/0x67
[<60153389>] prepend_name+0x1f/0x51
[<6015336a>] ? prepend_name+0x0/0x51
[<60152bbd>] prepend_path+0xb9/0x1d8
[<6003cb91>] ? um_set_signals+0x0/0x3b
[<60152e27>] d_path+0xce/0x115
[<60186805>] proc_pid_readlink+0xa1/0x17d
[<6012b192>] vfs_readlink+0xec/0xee
[<60138930>] ? touch_atime+0x0/0x162
[<60120351>] do_readlinkat+0xbe/0x13a
[<6011f92b>] sys_readlinkat+0x10/0x14
[<6002cabb>] handle_syscall+0x7b/0xaa
[<6002ca40>] ? handle_syscall+0x0/0xaa
[<6003f695>] userspace+0x289/0x4a5
[<6002a8e3>] fork_handler+0x56/0x5d
> diff --git a/arch/um/kernel/trap.c b/arch/um/kernel/trap.c
> index cbe924a0fa87..8a2e68d07de6 100644
> --- a/arch/um/kernel/trap.c
> +++ b/arch/um/kernel/trap.c
> @@ -330,20 +330,20 @@ unsigned long segv(struct faultinfo fi, unsigned long
> ip, int is_user,
> panic("Failed to sync kernel TLBs: %d", err);
> goto out;
> }
> - else if (current->mm == NULL) {
> - if (current->pagefault_disabled) {
> - if (!mc) {
> - show_regs(container_of(regs, struct pt_regs,
> regs));
> - panic("Segfault with pagefaults disabled but no
> mcontext");
> - }
> - if (!current->thread.segv_continue) {
> - show_regs(container_of(regs, struct pt_regs,
> regs));
> - panic("Segfault without recovery target");
> - }
> - mc_set_rip(mc, current->thread.segv_continue);
> - current->thread.segv_continue = NULL;
> - goto out;
> + else if (current->pagefault_disabled) {
> + if (!mc) {
> + show_regs(container_of(regs, struct pt_regs, regs));
> + panic("Segfault with pagefaults disabled but no
> mcontext");
> }
> + if (!current->thread.segv_continue) {
> + show_regs(container_of(regs, struct pt_regs, regs));
> + panic("Segfault without recovery target");
> + }
> + mc_set_rip(mc, current->thread.segv_continue);
> + current->thread.segv_continue = NULL;
> + goto out;
> + }
> + else if (current->mm == NULL) {
> show_regs(container_of(regs, struct pt_regs, regs));
> panic("Segfault with no mm");
> }
