linux-kernel - Re: [PATCH 1/1] KVM: inject data abort if instruction cannot be decoded

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:   Thu, 5 Sep 2019 09:56:44 +0100
From:   Peter Maydell <peter.maydell@...aro.org>
To:     Marc Zyngier <maz@...nel.org>
Cc:     Heinrich Schuchardt <xypron.glpk@....de>,
        James Morse <james.morse@....com>,
        Julien Thierry <julien.thierry@....com>,
        Suzuki K Pouloze <suzuki.poulose@....com>,
        Stefan Hajnoczi <stefanha@...hat.com>,
        Daniel P . Berrangé <berrange@...hat.com>,
        arm-mail-list <linux-arm-kernel@...ts.infradead.org>,
        kvmarm@...ts.cs.columbia.edu,
        lkml - Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] KVM: inject data abort if instruction cannot be decoded

On Thu, 5 Sep 2019 at 09:52, Marc Zyngier <maz@...nel.org> wrote:
>
> On Thu, 05 Sep 2019 09:16:54 +0100,
> Peter Maydell <peter.maydell@...aro.org> wrote:
> > This is true, but the problem is that barfing out to userspace
> > makes it harder to debug the guest because it means that
> > the VM is immediately destroyed, whereas AIUI if we
> > inject some kind of exception then (assuming you're set up
> > to do kernel-debug via gdbstub) you can actually examine
> > the offending guest code with a debugger because at least
> > your VM is still around to inspect...
>
> To Christoffer's point, I find the benefit a bit dubious. Yes, you get
> an exception, but the instruction that caused it may be completely
> legal (store with post-increment, for example), leading to an even
> more puzzled developer (that exception should never have been
> delivered the first place).

Right, but the combination of "host kernel prints a message
about an unsupported load/store insn" and "within-guest debug
dump/stack trace/etc" is much more useful than just having
"host kernel prints message" and "QEMU exits"; and it requires
about 3 lines of code change...

> I'm far more in favour of dumping the state of the access in the run
> structure (much like we do for a MMIO access) and let userspace do
> something about it (such as dumping information on the console or
> breaking). It could even inject an exception *if* the user has asked
> for it.

...whereas this requires agreement on a kernel-userspace API,
larger changes in the kernel, somebody to implement the userspace
side of things, and the user to update both the kernel and QEMU.
It's hard for me to see that the benefit here over the 3-line
approach really outweighs the extra effort needed. In practice
saying "we should do this" is saying "we're going to do nothing",
based on the historical record.

thanks
-- PMM