From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752367AbdJFOiF (ORCPT ); Fri, 6 Oct 2017 10:38:05 -0400 Received: from foss.arm.com ([217.140.101.70]:35038 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752314AbdJFOiD (ORCPT ); Fri, 6 Oct 2017 10:38:03 -0400 Subject: Re: [PATCH v2 04/10] KVM: arm/arm64: vgic-its: Check GITS_BASER Valid bit before saving tables To: Eric Auger , eric.auger.pro@gmail.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, marc.zyngier@arm.com, cdall@linaro.org, peter.maydell@linaro.org, wanghaibin.wang@huawei.com Cc: wu.wubin@huawei.com References: <1506518920-18571-1-git-send-email-eric.auger@redhat.com> <1506518920-18571-5-git-send-email-eric.auger@redhat.com> From: Andre Przywara Message-ID: Date: Fri, 6 Oct 2017 15:38:10 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <1506518920-18571-5-git-send-email-eric.auger@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 27/09/17 14:28, Eric Auger wrote: > At the moment we don't properly check the GITS_BASER.Valid > bit before saving the collection and device tables. > > On Collection table save() we use the gpa field whereas the Valid bit > should be used. On device table save() there is no check. This can > cause various bugs, among which a subsequent fault when accessing > the table in guest memory. > > Let's systematically check the Valid bit before doing anything. > > We also unifomize the code between save and restore. uniformize Apart from that: > Signed-off-by: Eric Auger Reviewed-by: Andre Przywara Cheers, Andre. > --- > virt/kvm/arm/vgic/vgic-its.c | 20 +++++++++++--------- > 1 file changed, 11 insertions(+), 9 deletions(-) > > diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c > index c1f7972..60ecf91 100644 > --- a/virt/kvm/arm/vgic/vgic-its.c > +++ b/virt/kvm/arm/vgic/vgic-its.c > @@ -2067,11 +2067,12 @@ static int vgic_its_device_cmp(void *priv, struct list_head *a, > static int vgic_its_save_device_tables(struct vgic_its *its) > { > const struct vgic_its_abi *abi = vgic_its_get_abi(its); > + u64 baser = its->baser_device_table; > struct its_device *dev; > int dte_esz = abi->dte_esz; > - u64 baser; > > - baser = its->baser_device_table; > + if (!(baser & GITS_BASER_VALID)) > + return 0; > > list_sort(NULL, &its->device_list, vgic_its_device_cmp); > > @@ -2217,17 +2218,17 @@ static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz) > static int vgic_its_save_collection_table(struct vgic_its *its) > { > const struct vgic_its_abi *abi = vgic_its_get_abi(its); > + u64 baser = its->baser_coll_table; > + gpa_t gpa = BASER_ADDRESS(baser); > struct its_collection *collection; > u64 val; > - gpa_t gpa; > size_t max_size, filled = 0; > int ret, cte_esz = abi->cte_esz; > > - gpa = BASER_ADDRESS(its->baser_coll_table); > - if (!gpa) > + if (!(baser & GITS_BASER_VALID)) > return 0; > > - max_size = GITS_BASER_NR_PAGES(its->baser_coll_table) * SZ_64K; > + max_size = GITS_BASER_NR_PAGES(baser) * SZ_64K; > > list_for_each_entry(collection, &its->collection_list, coll_list) { > ret = vgic_its_save_cte(its, collection, gpa, cte_esz); > @@ -2258,17 +2259,18 @@ static int vgic_its_save_collection_table(struct vgic_its *its) > static int vgic_its_restore_collection_table(struct vgic_its *its) > { > const struct vgic_its_abi *abi = vgic_its_get_abi(its); > + u64 baser = its->baser_coll_table; > int cte_esz = abi->cte_esz; > size_t max_size, read = 0; > gpa_t gpa; > int ret; > > - if (!(its->baser_coll_table & GITS_BASER_VALID)) > + if (!(baser & GITS_BASER_VALID)) > return 0; > > - gpa = BASER_ADDRESS(its->baser_coll_table); > + gpa = BASER_ADDRESS(baser); > > - max_size = GITS_BASER_NR_PAGES(its->baser_coll_table) * SZ_64K; > + max_size = GITS_BASER_NR_PAGES(baser) * SZ_64K; > > while (read < max_size) { > ret = vgic_its_restore_cte(its, gpa, cte_esz); >