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: Wed, 14 Feb 2024 12:55:11 -0800
From: Oliver Upton <oliver.upton@...ux.dev>
To: Marc Zyngier <maz@...nel.org>
Cc: kvmarm@...ts.linux.dev, kvm@...r.kernel.org,
	James Morse <james.morse@....com>,
	Suzuki K Poulose <suzuki.poulose@....com>,
	Zenghui Yu <yuzenghui@...wei.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 19/23] KVM: selftests: Add a minimal library for
 interacting with an ITS

On Wed, Feb 14, 2024 at 08:09:52PM +0000, Marc Zyngier wrote:
> > If the order of restore from userspace is CBASER, CWRITER, CREADR then
> > we **wind up replaying the entire command queue**. While insane, I'm
> > pretty sure it is legal for the guest to write garbage after the read
> > pointer has moved past a particular command index.
> > 
> > Fsck!!!
> 
> This is documented Documentation/virt/kvm/devices/arm-vgic-its.rst to
> some extent, and it is allowed for the guest to crap itself on behalf
> of userspace if the ordering isn't respected.

Ah, fair, I missed the documentation here. If we require userspace to
write CTLR last then we _should_ be fine, but damn is this a tricky set
of expectations.

> > So, how about we do this:
> > 
> >  - Provide a uaccess hook for CWRITER that changes the write-pointer
> >    without processing any commands
> > 
> >  - Assert an invariant that at any time CWRITER or CREADR are read from
> >    userspace that CREADR == CWRITER. Fail the ioctl and scream if that
> >    isn't the case, so that way we never need to worry about processing
> >    'in-flight' commands at the destination.
> 
> Are we guaranteed that we cannot ever see CWRITER != CREADR at VM
> dumping time? I'm not convinced that we cannot preempt the vcpu thread
> at the right spot, specially given that you can have an arbitrary
> large batch of commands to execute.
> 
> Just add a page-fault to the mix, and a signal pending. Pronto, you
> see a guest exit and you should be able to start dumping things
> without the ITS having processed much. I haven't tried, but that
> doesn't seem totally unlikely.

Well, we would need to run all userspace reads and writes through the
cmd_lock in this case, which is what we already do for the CREADR
uaccess hook. To me the 'racy' queue accessors only make sense for guest
accesses, since the driver is expecting to poll for completion in that
case.

Otherwise we decide the existing rules for restoring the ITS are fine
and I get to keep my funky driver :)

-- 
Thanks,
Oliver

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ