linux-kernel - Re: [PATCH 1/2] KVM: arm64: vgic: Forbid invalid userspace Redistributor accesses

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]

Message-ID: <c20865a267e44d1e2c0d52ce4e012263@kernel.org>
Date:   Tue, 17 Nov 2020 08:49:07 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     Zenghui Yu <yuzenghui@...wei.com>
Cc:     kvmarm@...ts.cs.columbia.edu, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, eric.auger@...hat.com,
        james.morse@....com, julien.thierry.kdev@...il.com,
        suzuki.poulose@....com, wanghaibin.wang@...wei.com,
        Keqian Zhu <zhukeqian1@...wei.com>
Subject: Re: [PATCH 1/2] KVM: arm64: vgic: Forbid invalid userspace
 Redistributor accesses

Hi Zenghui,

On 2020-11-16 14:57, Zenghui Yu wrote:
> Hi Marc,
> 
> On 2020/11/16 22:10, Marc Zyngier wrote:
>>> My take is that only if the "[Re]Distributor base address" is 
>>> specified
>>> in the system memory map, will the user-provided 
>>> kvm_device_attr.offset
>>> make sense. And we can then handle the access to the register which 
>>> is
>>> defined by "base address + offset".
>> 
>> I'd tend to agree, but it is just that this is a large change at -rc4.
>> I'd rather have a quick fix for 5.10, and a more invasive change for 
>> 5.11,
>> spanning all the possible vgic devices.
> 
> So you prefer fixing it by "return a value that doesn't have the Last
> bit set" for v5.10? I'm ok with it and can send v2 for it.

Cool. Thanks for that.

> Btw, looking again at the way we handle the user-reading of GICR_TYPER
> 
> 	vgic_mmio_read_v3r_typer(vcpu, addr, len)
> 
> it seems that @addr is actually the *offset* of GICR_TYPER (0x0008) and
> @addr is unlikely to be equal to last_rdist_typer, which is the *GPA* 
> of
> the last RD. Looks like the user-reading of GICR_TYPER.Last is always
> broken?

I think you are right. Somehow, we don't seem to track the index of
the RD in the region, so we can never compute the address of the RD
even if the base address is set.

Let's drop the reporting of Last for userspace for now, as it never
worked. If you post a patch addressing that quickly, I'll get it to
Paolo by the end of the week (there's another fix that needs merging).

Eric: do we have any test covering the userspace API?

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...