linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Xi Ruoyao <xry111@xry111.site>
Cc: Andreas Schwab <schwab@suse.de>,
	Ben Hutchings <ben@decadent.org.uk>,
	linux-mips@vger.kernel.org,  linux-kernel@vger.kernel.org,
	Jiaxun Yang <jiaxun.yang@flygoat.com>,
	 Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	libc-alpha@sourceware.org
Subject: Re: Strange EFAULT on mips64el returned by syscall when another thread is forking
Date: Wed, 24 Jan 2024 13:54:34 -0800	[thread overview]
Message-ID: <CAHk-=wgVrw+8P68Sy2krcc3QFbm_eu_DRs0-i7mct_0BDORZuA@mail.gmail.com> (raw)
In-Reply-To: <e8583a3ab0522b4e75ba0ada47b6f093b186fa81.camel@xry111.site>

On Wed, 24 Jan 2024 at 13:33, Xi Ruoyao <xry111@xry111.site> wrote:
>
> Re-posting the broken test case for Ben (I also added a waitpid call to
> prevent PID exhaustion):

Funky, funky.

>       ssize_t ret = read (fd, buf, 7);
>       if (ret == -1 && errno == EFAULT)
>         abort ();

So I think I have a clue:

> and the "interesting" aspects:
>
> 1. If I change the third parameter of "read" to any value >= 8, it no
> longer fails.  But it fails with any integer in [1, 8).

One change (the only one, really), is that now that MIPS uses
lock_mm_and_find_vma(), it also has this code:

        if (regs && !user_mode(regs)) {
                unsigned long ip = instruction_pointer(regs);
                if (!search_exception_tables(ip))
                        return false;
        }

in case the mmap trylock fails.

That code protects against the deadlock case of "we hold the mmap
lock, and take a kernel page fault due to a bug, and that page fault
happens to be to user space, and the page fault code then deadlocks on
the mmap lock".

It's a rare bug, but it's so nasty to debug that x86 has had that code
pretty much forever, and the lock_mm_and_find_vma() helper got it that
way. MIPS was clearly expecting kernel debugging to happen on other
platforms ;)

And I think the "fails with any integer in [1, 8)" is because the MIPS
"copy_from_user()" code is likely doing something special for those
small copies.

And I note that the MIPS extable.c code uses

        fixup = search_exception_tables(exception_epc(regs));

Note the difference: lock_mm_and_find_vma() uses
instruction_pointer(regs), extable.c uses exception_epc(regs).

The former is just "((regs)->cp0_epc)", while the latter is some
complex mess due to MIPS delay slots and isa16.

My *suspicion* is that instruction_pointer() needs to be fixed to do
the same full exception_epc() thing.

But honestly, I absolutely detest delay slots and refuse to touch
anything MIPS for that reason,.

And there could certainly be something else going on too. But that odd
size limitation, and the fact that it only happens on MIPS, does make
me think the above analysis is right.

I guess you could test it by changing the two cases of
'instruction_pointer(regs)' in mm/memory.c to use exception_epc(regs)
instead. It will only build on MIPS, but for *testing* that theory
out, it's fine.

Over to MIPS people..

                        Linus

  parent reply	other threads:[~2024-01-24 21:54 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-24 10:42 Strange EFAULT on mips64el returned by syscall when another thread is forking Xi Ruoyao
2024-01-24 11:59 ` Andreas Schwab
2024-01-24 12:49   ` Xi Ruoyao
2024-01-24 16:13     ` Xi Ruoyao
2024-01-24 21:32       ` Xi Ruoyao
2024-01-24 21:38         ` Xi Ruoyao
2024-01-24 21:54         ` Linus Torvalds [this message]
2024-01-24 22:10           ` Linus Torvalds
2024-01-24 22:42             ` Xi Ruoyao
2024-01-25  9:28             ` Jiaxun Yang
2024-01-26 12:33 ` Jiaxun Yang
2024-01-26 12:58   ` Xi Ruoyao
2024-01-26 18:00     ` Xi Ruoyao

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='CAHk-=wgVrw+8P68Sy2krcc3QFbm_eu_DRs0-i7mct_0BDORZuA@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=ben@decadent.org.uk \
    --cc=jiaxun.yang@flygoat.com \
    --cc=libc-alpha@sourceware.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=schwab@suse.de \
    --cc=tsbogend@alpha.franken.de \
    --cc=xry111@xry111.site \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).