linux-kernel - Re: [RFC] Have insn decoder functions return success/failure

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:   Sat, 24 Oct 2020 10:23:16 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     Masami Hiramatsu <mhiramat@...nel.org>, x86-ml <x86@...nel.org>,
        Joerg Roedel <jroedel@...e.de>,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: [RFC] Have insn decoder functions return success/failure

On Fri, Oct 23, 2020 at 05:12:49PM -0700, Andy Lutomirski wrote:
> I disagree.  A real CPU does exactly what I'm describing.  If I stick

A real modern CPU fetches up to 32 bytes insn window which it tries
to decode etc. I don't know, though, what it does when that fetch
encounters a fault - I will have to ask people. I'm not sure it would
even try to feed a shorter stream of bytes to the decoder but lemme
ask...

> 0xcc at the end of a page and a make the next page not-present, I get
> #BP, not #PF.  But if I stick 0x0F at the end of a page and mark the
> next page not-present, I get #PF.  If we're trying to decode an
> instruction in user memory, we can kludge it by trying to fetch 15
> bytes and handling -EFAULT by fetching fewer bytes, but that's gross
> and doesn't really have the right semantics.  What we actually want is
> to fetch up to the page boundary and try to decode it.  If it's a
> valid instruction or if it's definitely invalid, we're done.
> Otherwise we fetch across the page boundary.

We can do that but why would you put all that logic in the insn decoder?
Is that use case sooo important?

I mean, it would work that way anyway *even* *now* - the insn decoder
will tell you that the insn it decoded wasn't valid and you, as a
caller, know that you didn't fetch the whole 15 bytes so that means
that you still need to fetch some more. You've got all the relevant
information.

> Eventually we should wrap this whole mess up in an insn_decode_user()
> helper that does the right thing.

Oh sure, you can do that easily. Just put the logic which determines
that it copied a shorter buffer and that it attempts to decode the
shorter buffer first in it. Yah, that can work.

> And we can then make that helper
> extra fancy by getting PKRU and EPT-hacker-execute-only right, whereas
> we currently get these cases wrong.
> 
> Does this make sense?

Sure, but you could point me to those cases so that I can get a better
idea what they do exactly.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette