linux-kernel - Re: [PATCH 3/3] KVM: arm/arm64: vgic: Don't rely on the wrong pending table

Open Source and information security mailing list archives

Message-ID: <40c96640-49b3-942b-59f7-3f2f1592d8d6@redhat.com>
Date:   Tue, 29 Oct 2019 23:52:02 +0100
From:   Auger Eric <eric.auger@...hat.com>
To:     Zenghui Yu <yuzenghui@...wei.com>, Marc Zyngier <maz@...nel.org>
Cc:     suzuki.poulose@....com, linux-kernel@...r.kernel.org,
        james.morse@....com, linux-arm-kernel@...ts.infradead.org,
        wanghaibin.wang@...wei.com, kvmarm@...ts.cs.columbia.edu,
        julien.thierry.kdev@...il.com
Subject: Re: [PATCH 3/3] KVM: arm/arm64: vgic: Don't rely on the wrong pending
 table

Hi Zenghui,

On 10/29/19 2:31 PM, Zenghui Yu wrote:
> Hi Eric,
> 
> On 2019/10/29 20:49, Auger Eric wrote:
>> On 10/29/19 1:27 PM, Zenghui Yu wrote:
>>> okay, the remaining question is that in vgic_v3_save_pending_tables():
>>>
>>>      stored = val & (1U << bit_nr);
>>>      if (stored == irq->pending_latch)
>>>          continue;
>>>
>>>      if (irq->pending_latch)
>>>          val |= 1 << bit_nr;
>>>      else
>>>          val &= ~(1 << bit_nr);
>>>
>>> Do we really have a scenario where irq->pending_latch==false and
>>> stored==true (corresponds to the above "else") and then we clear
>>> pending status of this LPI in guest memory?
>>> I can not think out one now.
>>
>> if you save, restore and save again. On the 1st save the LPI may be
>> pending, it gets stored. On the second save the LPI may be not pending
>> anymore?
> 
> I assume you mean the "restore" by vgic_its_restore_ite().

yes that's what I meant

> 
> While restoring a LPI, we will sync the pending status from guest
> pending table (into the software pending_latch), and clear the
> corresponding bit in guest memory.
> See vgic_v3_lpi_sync_pending_status().
> 
> So on the second save, the LPI can be not pending, the guest pending
> table will also indicate not pending.

You're right; I did not remember vgic_v3_lpi_sync_pending_status (called
from vgic_its_restore_ite/vgic_add_lpi) "cleared the consumed data"
(44de9d683847  KVM: arm64: vgic-v3: vgic_v3_lpi_sync_pending_status).

So effectively after restore the pending table is zeroed and the above
code should be rewrittable in a more simple manner, ie. just update the
byte in case the pending_latch is set.

Nethertheless your patch indeed fixes an actual bug independently on
this cleanup, ie. the written byte may be incorrect if LPIs belonging to
this byte target different RDIST.

Thanks

Eric
> 
> 
> Thanks,
> Zenghui
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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