linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Michael Schmitz <schmitzmic@gmail.com>
Cc: geert@linux-m68k.org, linux-arch@vger.kernel.org,
	linux-m68k@lists.linux-m68k.org, torvalds@linux-foundation.org,
	schwab@linux-m68k.org
Subject: Re: [PATCH v4 0/3] m68k: Improved switch stack handling
Date: Sat, 17 Jul 2021 13:52:39 -0500	[thread overview]
Message-ID: <8735scvklk.fsf@disp2133> (raw)
In-Reply-To: <1b656c02-925c-c4ba-03d3-f56075cdfac5@gmail.com> (Michael Schmitz's message of "Sat, 17 Jul 2021 17:38:01 +1200")

Michael Schmitz <schmitzmic@gmail.com> writes:

> Am 16.07.2021 um 11:10 schrieb Michael Schmitz:
>> Eric,
>>
>> On 16/07/21 1:29 am, Eric W. Biederman wrote:
>>>
>>> I have been digging into this some more and I have found one place
>>> that I am having a challenge dealing with.
>>>
>>> In arch/m68k/fpsp040/skeleton.S there is an assembly version of
>>> copy_from_user that calls fpsp040_die when the bytes can not be read.
>>>
>>> Now fpsp040_die is just:
>>>
>>> /*
>>>   * This function is called if an error occur while accessing
>>>   * user-space from the fpsp040 code.
>>>   */
>>> asmlinkage void fpsp040_die(void)
>>> {
>>>     do_exit(SIGSEGV);
>>> }
>>> The problem here is the instruction emulation performed in the fpsp040
>>> code performs a very minimal saving of registers.  I don't think even
>>> the normal system call entry point registers that are saved are present
>>> at that point.
>>>
>>> Is there any chance you can help me figure out how to get a stack frame
>>> with all of the registers present before fpsp040_die is called?
>>
>> I suppose adding the following code (untested) to entry.S:
>>
>> ENTRY(fpsp040_die)
>>         SAVE_ALL_INT
>>         jbsr    fpsp040_die_c
>>         jra     ret_from_exception
>>
>> along with renaming above C entry point to fpsp040_die_c would add the
>> basic saved registers, but these would not necessarily reflect the state
>> of the processor when the fpsp040 trap was called. Is that what you're
>> after?
>
> I should have looked more closely at skeleton.S - most FPU exceptions
> handled there call trap_c the same way as is done for generic traps,
> i.e. SAVE_ALL_INT before, ret_from_exception after.
>
> Instead of adding code to entry.S, much better to add it in
> skeleton.S. I'll try to come up with a way to test this code path
> (calling fpsp040_die from the dz exception hander seems much the
> easiest way) to make sure this doesn't have side effects.
>
> Does do_exit() ever return?

No.  The function do_exit never returns.

If it is not too much difficulty I would be in favor of having the code
do force_sigsegv(SIGSEGV), instead of calling do_exit directly.

Looking at that code I have not been able to figure out the call paths
that get into skeleton.S.  I am not certain saving all of the registers
on an the exceptions that reach there make sense.  In practice I suspect
taking an exception is much more expensive than saving the registers so it
might not make any difference.  But this definitely looks like code that
is performance sensitive.

My sense when I was reading through skeleton.S was just one or two
registers were saved before the instruction emulation was called.

Eric


  reply	other threads:[~2021-07-17 18:52 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-23  0:21 [PATCH v4 0/3] m68k: Improved switch stack handling Michael Schmitz
2021-06-23  0:21 ` [PATCH v4 1/3] m68k: save extra registers on more syscall entry points Michael Schmitz
2021-06-23  0:21 ` [PATCH v4 2/3] m68k: correctly handle IO worker stack frame set-up Michael Schmitz
2021-06-23  0:21 ` [PATCH v4 3/3] m68k: track syscalls being traced with shallow user context stack Michael Schmitz
2021-07-25 10:05   ` Geert Uytterhoeven
2021-07-25 20:48     ` Michael Schmitz
2021-07-25 21:00       ` Linus Torvalds
2021-07-26 14:27         ` Greg Ungerer
2021-07-15 13:29 ` [PATCH v4 0/3] m68k: Improved switch stack handling Eric W. Biederman
2021-07-15 23:10   ` Michael Schmitz
2021-07-17  5:38     ` Michael Schmitz
2021-07-17 18:52       ` Eric W. Biederman [this message]
2021-07-17 20:09         ` Michael Schmitz
2021-07-17 23:04           ` Michael Schmitz
2021-07-18 10:47             ` Andreas Schwab
2021-07-18 19:47               ` Michael Schmitz
2021-07-18 20:59                 ` Brad Boyer
2021-07-19  3:15                   ` Michael Schmitz
2021-07-20 20:32             ` Eric W. Biederman
2021-07-20 22:16               ` Michael Schmitz
2021-07-22 14:49                 ` Eric W. Biederman
2021-07-23  4:23                   ` Michael Schmitz
2021-07-23 22:31                     ` Eric W. Biederman
2021-07-23 23:52                       ` Michael Schmitz
2021-07-24 12:05                         ` Andreas Schwab
2021-07-25  7:44                           ` Michael Schmitz
2021-07-25 10:12                             ` Brad Boyer
2021-07-26  2:00                               ` Michael Schmitz
2021-07-26 19:36                                 ` [RFC][PATCH] signal/m68k: Use force_sigsegv(SIGSEGV) in fpsp040_die Eric W. Biederman
2021-07-26 20:13                                   ` Andreas Schwab
2021-07-26 20:29                                     ` Eric W. Biederman
2021-07-26 21:25                                       ` Andreas Schwab
2021-07-26 20:29                                   ` Michael Schmitz
2021-07-26 21:08                                     ` [PATCH] " Eric W. Biederman
2021-08-25 15:56                                       ` Eric W. Biederman
2021-08-26 12:15                                       ` Geert Uytterhoeven
2021-07-25 11:53                             ` [PATCH v4 0/3] m68k: Improved switch stack handling Andreas Schwab

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=8735scvklk.fsf@disp2133 \
    --to=ebiederm@xmission.com \
    --cc=geert@linux-m68k.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    --cc=schmitzmic@gmail.com \
    --cc=schwab@linux-m68k.org \
    --cc=torvalds@linux-foundation.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 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).