linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch V2 00/20] x86/fpu: Clean up exception fixups and error handling in sigframe related code
@ 2021-09-07 19:56 Thomas Gleixner
  2021-09-07 20:04 ` Thomas Gleixner
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Gleixner @ 2021-09-07 19:56 UTC (permalink / raw)
  To: LKML
  Cc: x86, Al Viro, Linus Torvalds, Tony Luck, Song Liu,
	Alexei Starovoitov, Daniel Borkmann, Peter Ziljstra

A recent discussion [1] about hardware poisoning unearthed some short
comings in the error handling of the sigframe related FPU code:

  - The error exit for exceptions other than #PF is obfuscated

  - The error code return values of the various functions are pointless
    because all callers just care about success or failure and the error
    codes are never propagated to user space.

  - Some of the buffer clearing happens needlessly inside of page fault
    disabled regions.

The discussion around V1 of this series which can be found here:

  https://lore.kernel.org/r/20210830154702.247681585@linutronix.de

identified a few more issues especially in the area of exception fixups:

  - The MCE aware exception fixup is inconsistent and confusing especially
    in copy_mc_64.c. It uses a fixup function which stores the trap number
    in regs->ax just to overwrite regs->ax at the callsite specific fixup.

The following series cleans this up. The resulting excecutable code is
slightly smaller with that.

It's also available in git:

    git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/fpu

Changes vs. V1:

  - Deduplicate the exception table related code

  - Change the exception table store to use a fixup type identifier instead
    of a function pointer. That allows to use the same fixup function for
    different types and avoids adding new global functions and exports to
    solve the identified issues. This makes the cleanup of the odd fixup
    functions in copy_mc_64 and the fpu code simpler

  - Make copy_mc_64 and FPU code use the new fixup type mechanics

  - Remove #MC handling from the various *SAVE functions which write the
    FPU registers to the user space sigframe as these can't raise #MC
    according to Tony.

  - Address a few review comments and adjust the patches to the new
    exception table mechanism.

Thanks,

	tglx

[1] https://lore.kernel.org/r/87r1edgs2w.ffs@tglx

---
 arch/x86/ia32/ia32_signal.c                |   14 +-
 arch/x86/include/asm/asm.h                 |   49 ++++-----
 arch/x86/include/asm/extable.h             |   44 +++++---
 arch/x86/include/asm/extable_fixup_types.h |   22 ++++
 arch/x86/include/asm/fpu/internal.h        |   84 ++++++++++------
 arch/x86/include/asm/msr.h                 |    4 
 arch/x86/include/asm/segment.h             |    2 
 arch/x86/kernel/cpu/mce/core.c             |   40 ++------
 arch/x86/kernel/cpu/mce/internal.h         |   14 --
 arch/x86/kernel/cpu/mce/severity.c         |   22 ++--
 arch/x86/kernel/fpu/signal.c               |  145 +++++++++++++++--------------
 arch/x86/kernel/signal.c                   |   18 +--
 arch/x86/lib/copy_mc_64.S                  |    8 -
 arch/x86/mm/extable.c                      |  131 ++++++++++----------------
 arch/x86/net/bpf_jit_comp.c                |   11 --
 scripts/sorttable.c                        |    4 
 16 files changed, 302 insertions(+), 310 deletions(-)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch V2 00/20] x86/fpu: Clean up exception fixups and error handling in sigframe related code
  2021-09-07 19:56 [patch V2 00/20] x86/fpu: Clean up exception fixups and error handling in sigframe related code Thomas Gleixner
@ 2021-09-07 20:04 ` Thomas Gleixner
  2021-09-07 20:07   ` Luck, Tony
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Gleixner @ 2021-09-07 20:04 UTC (permalink / raw)
  To: LKML
  Cc: x86, Al Viro, Linus Torvalds, Tony Luck, Song Liu,
	Alexei Starovoitov, Daniel Borkmann, Peter Ziljstra

On Tue, Sep 07 2021 at 21:56, Thomas Gleixner wrote:

Please ignore. My quilt scripts went berserk for some weird reason.

I'll resend with proper threading after I figured it out.

Thanks and sorry for the noise

        tglx
  

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [patch V2 00/20] x86/fpu: Clean up exception fixups and error handling in sigframe related code
  2021-09-07 20:04 ` Thomas Gleixner
@ 2021-09-07 20:07   ` Luck, Tony
  2021-09-07 20:18     ` Thomas Gleixner
  0 siblings, 1 reply; 13+ messages in thread
From: Luck, Tony @ 2021-09-07 20:07 UTC (permalink / raw)
  To: Thomas Gleixner, LKML
  Cc: x86, Al Viro, Linus Torvalds, Song Liu, Alexei Starovoitov,
	Daniel Borkmann, Peter Ziljstra

> Please ignore. My quilt scripts went berserk for some weird reason.

Was it just the email post that was bad? I.e. are the patches in:

	git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/fpu

ok?  I just compiled that, should I boot it, or wait?

-Tony
 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [patch V2 00/20] x86/fpu: Clean up exception fixups and error handling in sigframe related code
  2021-09-07 20:07   ` Luck, Tony
@ 2021-09-07 20:18     ` Thomas Gleixner
  2021-09-07 21:13       ` Luck, Tony
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Gleixner @ 2021-09-07 20:18 UTC (permalink / raw)
  To: Luck, Tony, LKML
  Cc: x86, Al Viro, Linus Torvalds, Song Liu, Alexei Starovoitov,
	Daniel Borkmann, Peter Ziljstra

On Tue, Sep 07 2021 at 20:07, Tony Luck wrote:

>> Please ignore. My quilt scripts went berserk for some weird reason.
>
> Was it just the email post that was bad? I.e. are the patches in:
>
> 	git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/fpu
>
> ok?  I just compiled that, should I boot it, or wait?

Just the mail script went south. The git tree is fine.

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [patch V2 00/20] x86/fpu: Clean up exception fixups and error handling in sigframe related code
  2021-09-07 20:18     ` Thomas Gleixner
@ 2021-09-07 21:13       ` Luck, Tony
  2021-09-07 21:55         ` Thomas Gleixner
  0 siblings, 1 reply; 13+ messages in thread
From: Luck, Tony @ 2021-09-07 21:13 UTC (permalink / raw)
  To: Thomas Gleixner, LKML
  Cc: x86, Al Viro, Linus Torvalds, Song Liu, Alexei Starovoitov,
	Daniel Borkmann, Peter Ziljstra

>> Was it just the email post that was bad? I.e. are the patches in:
>>
>> 	git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/fpu
>>
>> ok?  I just compiled that, should I boot it, or wait?
>
> Just the mail script went south. The git tree is fine.

What else is in that tree?  My kernel doesn't boot (fails to find root filesystem device).
Same .config boots Linus latest (HEAD= 0bcfe68b876 Revert "memcg: enable accounting for pollfd and select bits arrays")

-Tony

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [patch V2 00/20] x86/fpu: Clean up exception fixups and error handling in sigframe related code
  2021-09-07 21:13       ` Luck, Tony
@ 2021-09-07 21:55         ` Thomas Gleixner
  2021-09-07 22:37           ` Thomas Gleixner
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Gleixner @ 2021-09-07 21:55 UTC (permalink / raw)
  To: Luck, Tony, LKML
  Cc: x86, Al Viro, Linus Torvalds, Song Liu, Alexei Starovoitov,
	Daniel Borkmann, Peter Ziljstra

On Tue, Sep 07 2021 at 21:13, Tony Luck wrote:

>>> Was it just the email post that was bad? I.e. are the patches in:
>>>
>>> 	git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/fpu
>>>
>>> ok?  I just compiled that, should I boot it, or wait?
>>
>> Just the mail script went south. The git tree is fine.
>
> What else is in that tree?  My kernel doesn't boot (fails to find root filesystem device).
> Same .config boots Linus latest (HEAD= 0bcfe68b876 Revert "memcg: enable accounting for pollfd and select bits arrays")

Huch? That tree is based on 0bcfe68b876 and it just has those 20 patches
on top which should not at all interfere with your root filesystem
device. Let me verify.

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [patch V2 00/20] x86/fpu: Clean up exception fixups and error handling in sigframe related code
  2021-09-07 21:55         ` Thomas Gleixner
@ 2021-09-07 22:37           ` Thomas Gleixner
  2021-09-07 23:20             ` Linus Torvalds
  2021-09-08  4:06             ` Luck, Tony
  0 siblings, 2 replies; 13+ messages in thread
From: Thomas Gleixner @ 2021-09-07 22:37 UTC (permalink / raw)
  To: Luck, Tony, LKML
  Cc: x86, Al Viro, Linus Torvalds, Song Liu, Alexei Starovoitov,
	Daniel Borkmann, Peter Ziljstra

On Tue, Sep 07 2021 at 23:55, Thomas Gleixner wrote:
> On Tue, Sep 07 2021 at 21:13, Tony Luck wrote:
>
>>>> Was it just the email post that was bad? I.e. are the patches in:
>>>>
>>>> 	git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/fpu
>>>>
>>>> ok?  I just compiled that, should I boot it, or wait?
>>>
>>> Just the mail script went south. The git tree is fine.
>>
>> What else is in that tree?  My kernel doesn't boot (fails to find root filesystem device).
>> Same .config boots Linus latest (HEAD= 0bcfe68b876 Revert "memcg: enable accounting for pollfd and select bits arrays")
>
> Huch? That tree is based on 0bcfe68b876 and it just has those 20 patches
> on top which should not at all interfere with your root filesystem
> device. Let me verify.

I lost connection to my test machines. Will continue tomorrow morning.

/me mumbles unprintable curses about living in a developing country...

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch V2 00/20] x86/fpu: Clean up exception fixups and error handling in sigframe related code
  2021-09-07 22:37           ` Thomas Gleixner
@ 2021-09-07 23:20             ` Linus Torvalds
  2021-09-07 23:22               ` Linus Torvalds
  2021-09-08  4:06             ` Luck, Tony
  1 sibling, 1 reply; 13+ messages in thread
From: Linus Torvalds @ 2021-09-07 23:20 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Luck, Tony, LKML, x86, Al Viro, Song Liu, Alexei Starovoitov,
	Daniel Borkmann, Peter Ziljstra

On Tue, Sep 7, 2021 at 3:37 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> /me mumbles unprintable curses about living in a developing country...

Germany has more ISDN connections than anywhere else in the world! How
can you possibly complain?

             Linus

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch V2 00/20] x86/fpu: Clean up exception fixups and error handling in sigframe related code
  2021-09-07 23:20             ` Linus Torvalds
@ 2021-09-07 23:22               ` Linus Torvalds
  2021-09-08 13:22                 ` Thomas Gleixner
  0 siblings, 1 reply; 13+ messages in thread
From: Linus Torvalds @ 2021-09-07 23:22 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Luck, Tony, LKML, x86, Al Viro, Song Liu, Alexei Starovoitov,
	Daniel Borkmann, Peter Ziljstra

On Tue, Sep 7, 2021 at 4:20 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Germany has more ISDN connections than anywhere else in the world! How
> can you possibly complain?

Actually, I take that back. Apparently DT finally got rid of it last
year. My bad.

           Linus

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [patch V2 00/20] x86/fpu: Clean up exception fixups and error handling in sigframe related code
  2021-09-07 22:37           ` Thomas Gleixner
  2021-09-07 23:20             ` Linus Torvalds
@ 2021-09-08  4:06             ` Luck, Tony
  2021-09-08  5:14               ` Thomas Gleixner
  1 sibling, 1 reply; 13+ messages in thread
From: Luck, Tony @ 2021-09-08  4:06 UTC (permalink / raw)
  To: Thomas Gleixner, LKML
  Cc: x86, Al Viro, Linus Torvalds, Song Liu, Alexei Starovoitov,
	Daniel Borkmann, Peter Ziljstra

>> Huch? That tree is based on 0bcfe68b876 and it just has those 20 patches
>> on top which should not at all interfere with your root filesystem
>> device. Let me verify.
>
> I lost connection to my test machines. Will continue tomorrow morning.

To save you some time I ran a bisect. It says the wheels fall off the bus at
patch 13/20

$ git bisect bad
43bce826b58940bd3143f110d36f5901d009e527 is the first bad commit
commit 43bce826b58940bd3143f110d36f5901d009e527
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Mon Aug 30 18:27:25 2021 +0200

    x86/fpu/signal: Move xstate clearing out of copy_fpregs_to_sigframe()

    When the direct saving of the FPU registers to the user space sigframe
    fails, copy_fpregs_to_sigframe() attempts to clear the user buffer.

    The most likely reason for such a fail is a page fault. As
    copy_fpregs_to_sigframe() is invoked with pagefaults disabled the chance
    that __clear_user() succeeds is minuscule.

    Move the clearing out into the caller which replaces the
    fault_in_pages_writeable() in that error handling path.

    The return value confusion will be cleaned up separately.

    Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

:040000 040000 a7dce9444541186dcc30f21c9d0416d48f215507 71056cf4baa014ca33ab4861b0aca76b154979bf M arch

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [patch V2 00/20] x86/fpu: Clean up exception fixups and error handling in sigframe related code
  2021-09-08  4:06             ` Luck, Tony
@ 2021-09-08  5:14               ` Thomas Gleixner
  2021-09-08 13:21                 ` Thomas Gleixner
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Gleixner @ 2021-09-08  5:14 UTC (permalink / raw)
  To: Luck, Tony, LKML
  Cc: x86, Al Viro, Linus Torvalds, Song Liu, Alexei Starovoitov,
	Daniel Borkmann, Peter Ziljstra

On Wed, Sep 08 2021 at 04:06, Tony Luck wrote:

>>> Huch? That tree is based on 0bcfe68b876 and it just has those 20 patches
>>> on top which should not at all interfere with your root filesystem
>>> device. Let me verify.
>>
>> I lost connection to my test machines. Will continue tomorrow morning.
>
> To save you some time I ran a bisect. It says the wheels fall off the bus at
> patch 13/2

Yes, but that's not the real culprit. That's just the messenger.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [patch V2 00/20] x86/fpu: Clean up exception fixups and error handling in sigframe related code
  2021-09-08  5:14               ` Thomas Gleixner
@ 2021-09-08 13:21                 ` Thomas Gleixner
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Gleixner @ 2021-09-08 13:21 UTC (permalink / raw)
  To: Luck, Tony, LKML
  Cc: x86, Al Viro, Linus Torvalds, Song Liu, Alexei Starovoitov,
	Daniel Borkmann, Peter Ziljstra

On Wed, Sep 08 2021 at 07:14, Thomas Gleixner wrote:
> On Wed, Sep 08 2021 at 04:06, Tony Luck wrote:
>
>>>> Huch? That tree is based on 0bcfe68b876 and it just has those 20 patches
>>>> on top which should not at all interfere with your root filesystem
>>>> device. Let me verify.
>>>
>>> I lost connection to my test machines. Will continue tomorrow morning.
>>
>> To save you some time I ran a bisect. It says the wheels fall off the bus at
>> patch 13/2
>
> Yes, but that's not the real culprit. That's just the messenger.

This patch triggers it because it has the left over #PF check from
V1. But then I removed the MCE safe fixup from the *SAVE functions, got
distracted and tested a stale kernel... I'll send a V3 in a minute

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch V2 00/20] x86/fpu: Clean up exception fixups and error handling in sigframe related code
  2021-09-07 23:22               ` Linus Torvalds
@ 2021-09-08 13:22                 ` Thomas Gleixner
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Gleixner @ 2021-09-08 13:22 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Luck, Tony, LKML, x86, Al Viro, Song Liu, Alexei Starovoitov,
	Daniel Borkmann, Peter Ziljstra

On Tue, Sep 07 2021 at 16:22, Linus Torvalds wrote:
> On Tue, Sep 7, 2021 at 4:20 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> Germany has more ISDN connections than anywhere else in the world! How
>> can you possibly complain?
>
> Actually, I take that back. Apparently DT finally got rid of it last
> year. My bad.

That's the problem actually. ISDN was slow but very reliable.

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-09-08 13:22 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-07 19:56 [patch V2 00/20] x86/fpu: Clean up exception fixups and error handling in sigframe related code Thomas Gleixner
2021-09-07 20:04 ` Thomas Gleixner
2021-09-07 20:07   ` Luck, Tony
2021-09-07 20:18     ` Thomas Gleixner
2021-09-07 21:13       ` Luck, Tony
2021-09-07 21:55         ` Thomas Gleixner
2021-09-07 22:37           ` Thomas Gleixner
2021-09-07 23:20             ` Linus Torvalds
2021-09-07 23:22               ` Linus Torvalds
2021-09-08 13:22                 ` Thomas Gleixner
2021-09-08  4:06             ` Luck, Tony
2021-09-08  5:14               ` Thomas Gleixner
2021-09-08 13:21                 ` Thomas Gleixner

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).