Subject: [Bug 214913] [xfstests generic/051] BUG: Kernel NULL pointer dereference on read at 0x00000108 NIP [c0000000000372e4] tm_cgpr_active+0x14/0x40

From: [email-masked]
Date: Mon, 12 Dec 2022 03:52:36 +0000

https://bugzilla.kernel.org/show_bug.cgi?id=214913

--- Comment #8 from [email-masked] ---
On Sun Dec 11, 2022 at 11:19 PM AEST,  wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=214913
>
> --- Comment #7 from Zorro Lang ([email-masked]) ---
> (In reply to Michael Ellerman from comment #5)
> > Sorry I don't have any idea which commit could have fixed this.
> > 
> > The process that crashed was "fsstress", do you know if it uses io_uring?
>
> Yes, fsstress has io_uring read/write operations. And from the kernel .config
> file(as attachment), the CONFIG_IO_URING=y

The task being dumped seems like it's lost its task->thread.regs. The
NULL pointer is here:

int tm_cgpr_active(struct task_struct *target, const struct user_regset
*regset)
{
        if (!cpu_has_feature(CPU_FTR_TM))
                return -ENODEV;

        if (!MSR_TM_ACTIVE(target->thread.regs->msr))
                return 0;

        return regset->n;
}

On that regs->msr deref. r9 contains the regs pointer.

The kernel attempt to read user page - exploit attempt? message is
I think a red herring it's coming up because of the NULL deref I
think (I thought we fixed that).

Anyway I'm not sure how we could lose regs, all user threads should
have them set to non-NULL. It doesn't look like we can collect threads
for dumping before we have called copy_thread(), which is where they
get thread.regs set. AFAIK it's not supposed to change after that.

Would you be able to try this patch, hopefully it catches the problem
thread on the exit side, and gives a clue why regs is NULL.

Thanks,
Nick

---

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 6a11025e5850..ece63b3d2304 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1898,9 +1898,21 @@ static int fill_note_info(struct elfhdr *elf, int phdrs,
        /*
         * Now fill in each thread's information.
         */
-       for (t = info->thread; t != NULL; t = t->next)
+       for (t = info->thread; t != NULL; t = t->next) {
+               if (!t->task) {
+                       WARN_ON(1);
+                       printk("core info lost task\n");
+                       continue;
+               }
+               if (!t->task->thread.regs) {
+                       WARN_ON(1);
+                       printk("lost regs pid:%d (current->pid:%d)\n",
t->task->pid, current->pid);
+                       continue;
+               }
+
                if (!fill_thread_core_info(t, view, cprm->siginfo->si_signo,
info))
                        return 0;
+       }

        /*
         * Fill in the two process-wide notes.
diff --git a/kernel/exit.c b/kernel/exit.c
index 35e0a31a0315..6820fe333081 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -366,6 +366,8 @@ static void coredump_task_exit(struct task_struct *tsk)
        if (core_state) {
                struct core_thread self;

+               WARN_ON(!current->thread.regs);
+
                self.task = current;
                if (self.task->flags & PF_SIGNALED)
                        self.next = xchg(&core_state->dumper.next, &self);

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.