linux-kernel - Re: [patch 01/10] x86/fpu/signal: Clarify exception handling in restore_fpregs_from

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 30 Aug 2021 14:26:12 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Al Viro <viro@...iv.linux.org.uk>,
        Dan Williams <dan.j.williams@...el.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...en8.de>,
        LKML <linux-kernel@...r.kernel.org>,
        "the arch/x86 maintainers" <x86@...nel.org>
Subject: Re: [patch 01/10] x86/fpu/signal: Clarify exception handling in restore_fpregs_from_user()

On Mon, Aug 30, 2021 at 2:07 PM Al Viro <viro@...iv.linux.org.uk> wrote:
>
> Incidentally, why do we bother with negation in those?  Why not have
> user_insn(), XSTATE_OP() and kernel_insn_err() return 0 or trap number...

I really wish we didn't have that odd _ASM_EXTABLE_FAULT/
ex_handler_fault() special case at all.

It's *very* confusing, and it actually seems to be mis-used. It looks
like the "copy_mc_fragile" code uses it by mistake, and doesn't
actually want that "modify %%rax" behavior of that exception handler
AT ALL.

If I read that code correctly, it almost by mistake doesn't actually
care, and will overwrite %%rax with the right result, but it doesn't
look like the "fault code in %eax" was ever *intentional*. There's no
mention of it.

Maybe I'm misreading that code, but I look at it and just go "Whaa?"

The code in user_insn() clearly *does* use that fault number (and, as
you say, inverts it for some reason), but I wonder how much it really
cares? Could we get rid of it, and just set a fixed error code?

I only checked one user, but that one didn't actually care which fault
it was, it only cared about fault-vs-no-fault.

                Linus

             Linus