From: "Nicholas Piggin" <npiggin@gmail.com>
To: "Jens Axboe" <axboe@kernel.dk>,
"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH] powerpc: don't try to copy ppc for task with NULL pt_regs
Date: Mon, 27 Mar 2023 16:36:30 +1000 [thread overview]
Message-ID: <CRGYHQ3C77DV.1PXS812TV997N@bobo> (raw)
In-Reply-To: <d9f63344-fe7c-56ae-b420-4a1a04a2ae4c@kernel.dk>
On Mon Mar 27, 2023 at 8:15 AM AEST, Jens Axboe wrote:
> Powerpc sets up PF_KTHREAD and PF_IO_WORKER with a NULL pt_regs, which
> from my (arguably very short) checking is not commonly done for other
> archs. This is fine, except when PF_IO_WORKER's have been created and
> the task does something that causes a coredump to be generated. Then we
> get this crash:
Hey Jens,
Thanks for the testing and the patch.
I think your patch would work, but I'd be inclined to give the IO worker
a pt_regs so it looks more like other archs and a regular user thread.
Your IO worker bug reminded me to resurrect some copy_thread patches I
had and I think they should do that
https://lists.ozlabs.org/pipermail/linuxppc-dev/2023-March/256271.html
I wouldn't ask you to test it until I've at least tried, do you have a
test case that triggers this?
Thanks,
Nick
>
> Kernel attempted to read user page (160) - exploit attempt? (uid: 1000)
> BUG: Kernel NULL pointer dereference on read at 0x00000160
> Faulting instruction address: 0xc0000000000c3a60
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=32 NUMA pSeries
> Modules linked in: bochs drm_vram_helper drm_kms_helper xts binfmt_misc ecb ctr syscopyarea sysfillrect cbc sysimgblt drm_ttm_helper aes_generic ttm sg libaes evdev joydev virtio_balloon vmx_crypto gf128mul drm dm_mod fuse loop configfs drm_panel_orientation_quirks ip_tables x_tables autofs4 hid_generic usbhid hid xhci_pci xhci_hcd usbcore usb_common sd_mod
> CPU: 1 PID: 1982 Comm: ppc-crash Not tainted 6.3.0-rc2+ #88
> Hardware name: IBM pSeries (emulated by qemu) POWER9 (raw) 0x4e1202 0xf000005 of:SLOF,HEAD hv:linux,kvm pSeries
> NIP: c0000000000c3a60 LR: c000000000039944 CTR: c0000000000398e0
> REGS: c0000000041833b0 TRAP: 0300 Not tainted (6.3.0-rc2+)
> MSR: 800000000280b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 88082828 XER: 200400f8
> CFAR: c0000000000c386c DAR: 0000000000000160 DSISR: 40000000 IRQMASK: 0
> GPR00: c000000000039920 c000000004183650 c00000000175d600 c0000000040f9800
> GPR04: 0000000000000160 0000000000000008 c000000000039920 0000000000000000
> GPR08: 0000000000000000 0000000000000000 00000003fe060000 0000000000002000
> GPR12: c0000000000398e0 c0000003fffff200 c0000000015edbc0 c00000000ba2f648
> GPR16: c00000000ba2f600 c000000001616ea8 0000000000000004 00000000ffffffff
> GPR20: 0000000000000048 c000000004183918 c000000001410f00 c000000001410ef8
> GPR24: c0000000040f9800 c0000000040f9800 c0000000041837b8 c0000000000398e0
> GPR28: c00000000cc4cb80 c0000000040f9800 0000000000000008 0000000000000008
> NIP [c0000000000c3a60] memcpy_power7+0x200/0x7d0
> LR [c000000000039944] ppr_get+0x64/0xb0
> Call Trace:
> [c000000004183650] [c000000000039920] ppr_get+0x40/0xb0 (unreliable)
> [c000000004183690] [c0000000001e5e80] __regset_get+0x180/0x1f0
> [c000000004183700] [c0000000001e5f94] regset_get_alloc+0x64/0x90
> [c000000004183740] [c0000000007ae638] elf_core_dump+0xb98/0x1b60
> [c0000000041839c0] [c0000000007bb564] do_coredump+0x1c34/0x24a0
> [c000000004183ba0] [c0000000001acf0c] get_signal+0x71c/0x1410
> [c000000004183ce0] [c0000000000228a0] do_notify_resume+0x140/0x6f0
> [c000000004183db0] [c0000000000353bc] interrupt_exit_user_prepare_main+0x29c/0x320
> [c000000004183e20] [c00000000003579c] interrupt_exit_user_prepare+0x6c/0xa0
> [c000000004183e50] [c00000000000c6f4] interrupt_return_srr_user+0x8/0x138
> --- interrupt: 300 at 0x183ee09e0
> NIP: 0000000183ee09e0 LR: 0000000183ee09dc CTR: 800000000280f033
> REGS: c000000004183e80 TRAP: 0300 Not tainted (6.3.0-rc2+)
> MSR: 800000000000d033 <SF,EE,PR,ME,IR,DR,RI,LE> CR: 22002848 XER: 000000f8
> CFAR: 00007ffe6d746aa8 DAR: 0000000000000000 DSISR: 42000000 IRQMASK: 0
> GPR00: 0000000183ee09dc 00007ffff20d37c0 0000000183f07f00 0000000000000000
> GPR04: 0000000000000000 00007ffff20d37a8 0000000000000000 00007ffe6d9eae00
> GPR08: 00007ffff20d3710 0000000000000000 0000000000000000 0000000000000000
> GPR12: 0000000000000000 00007ffe6d9eae00 0000000000000000 0000000000000000
> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000183ee0860
> GPR24: 00007ffe6d9df820 00007ffe6d9e0000 00007ffff20d7d98 0000000000000001
> GPR28: 0000000183ee0c60 00007ffff20d7924 00007ffff20d7820 0000000000000000
> NIP [0000000183ee09e0] 0x183ee09e0
> LR [0000000183ee09dc] 0x183ee09dc
> --- interrupt: 300
> Code: f9030018 38630020 409f001c e8040000 e8c40008 38840010 f8030000 f8c30008 38630010 78a50720 7cb01120 409c001c <80040000> 80c40004 38840008 90030000
> ---[ end trace 0000000000000000 ]---
>
> note: ppc-crash[1982] exited with irqs disabled
>
> because ppr_get() is trying to copy from a PF_IO_WORKER with a NULL
> pt_regs.
>
> Check for a valid pt_regs in both ppc_get/ppr_set, and return an error
> if not set. The actual error value doesn't seem to be important here,
> so just pick -EINVAL.
>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>
> diff --git a/arch/powerpc/kernel/ptrace/ptrace-view.c b/arch/powerpc/kernel/ptrace/ptrace-view.c
> index 2087a785f05f..80b699dd0d7f 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace-view.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace-view.c
> @@ -290,6 +290,8 @@ static int gpr_set(struct task_struct *target, const struct user_regset *regset,
> static int ppr_get(struct task_struct *target, const struct user_regset *regset,
> struct membuf to)
> {
> + if (!target->thread.regs)
> + return -EINVAL;
> return membuf_write(&to, &target->thread.regs->ppr, sizeof(u64));
> }
>
> @@ -297,6 +299,8 @@ static int ppr_set(struct task_struct *target, const struct user_regset *regset,
> unsigned int pos, unsigned int count, const void *kbuf,
> const void __user *ubuf)
> {
> + if (!target->thread.regs)
> + return -EINVAL;
> return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> &target->thread.regs->ppr, 0, sizeof(u64));
> }
>
> --
> Jens Axboe
next prev parent reply other threads:[~2023-03-27 6:37 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-26 22:15 [PATCH] powerpc: don't try to copy ppc for task with NULL pt_regs Jens Axboe
2023-03-27 6:36 ` Nicholas Piggin [this message]
2023-03-27 10:26 ` Christophe Leroy
2023-03-28 6:16 ` Nicholas Piggin
2023-03-28 11:47 ` Michael Ellerman
2023-03-30 8:44 ` Christophe Leroy
2023-03-27 12:42 ` Jens Axboe
2023-03-27 13:54 ` Michael Ellerman
2023-03-27 16:10 ` Jens Axboe
2023-03-28 11:32 ` Michael Ellerman
2023-03-28 12:39 ` Jens Axboe
2023-04-06 0:26 ` Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CRGYHQ3C77DV.1PXS812TV997N@bobo \
--to=npiggin@gmail.com \
--cc=axboe@kernel.dk \
--cc=linuxppc-dev@lists.ozlabs.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.