From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 89E29C27C4F for ; Fri, 21 Jun 2024 06:39:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To: Content-Transfer-Encoding:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=i/rGr0WknPRR6M1jYoCS1tGH+uD6OvxVso60P4Qdv+c=; b=rPTQENermZzsZ4HSoqUhN/UpJK OmXfoF2KRdg/EHZ0HVGsJ698h/oBT9mV4LVMDtJHLDlrcQQ0merYXuaVYtzqww5CkHG/dWiBthQbB UUAMbHbWX5iyMIrTIv9H7+L4ovg66+tU0exgxbh2vldCMqEoH12DF5l4hkIjX1aMfFEBiR9bF+p30 6elmVhjdneOBpY8MtIj2Fv2RQEaCVlBtqbLLdc0Cps1N1l5h9Cf26ONdJoNYn0HLQtP2KW3QqQ7AD erSRBb1RrqkjWOC03xIAvLEqbcMSnoObzlYFZ4Q+C3joQQxUxj8hXjiBvQx8Elt5AkHS6agGJkk1B q416hp+Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sKXvt-000000080qQ-3oBs; Fri, 21 Jun 2024 06:39:37 +0000 Received: from out-188.mta0.migadu.com ([2001:41d0:1004:224b::bc]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sKXvp-000000080pG-2IGx for linux-arm-kernel@lists.infradead.org; Fri, 21 Jun 2024 06:39:35 +0000 X-Envelope-To: jiangkunkun@huawei.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1718951970; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=i/rGr0WknPRR6M1jYoCS1tGH+uD6OvxVso60P4Qdv+c=; b=uz//xHnyvplYyjijHTY6adMHi+pCmk9Jy8g9YToZ8QvYqkm6/bid8DjFvEQTGGor7bt1D0 LakgR05jXzpmTSobKau3KnS/tn2NOa0hHf88A2d50iCpgPbon1ufY5IqWYyLXk1OwLebBw IVF1PIhmwHKFCmg1En29Fid8gAiUI6A= X-Envelope-To: maz@kernel.org X-Envelope-To: james.morse@arm.com X-Envelope-To: suzuki.poulose@arm.com X-Envelope-To: yuzenghui@huawei.com X-Envelope-To: catalin.marinas@arm.com X-Envelope-To: will@kernel.org X-Envelope-To: rdunlap@infradead.org X-Envelope-To: eauger@redhat.com X-Envelope-To: linux-arm-kernel@lists.infradead.org X-Envelope-To: kvmarm@lists.linux.dev X-Envelope-To: wanghaibin.wang@huawei.com X-Envelope-To: lishusen2@huawei.com Date: Fri, 21 Jun 2024 06:39:25 +0000 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Oliver Upton To: Kunkun Jiang Cc: Marc Zyngier , James Morse , Suzuki K Poulose , Zenghui Yu , Catalin Marinas , Will Deacon , Randy Dunlap , Eric Auger , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, wanghaibin.wang@huawei.com, lishusen2@huawei.com Subject: Re: [PATCH 1/3] KVM: arm64: vgic-its: Add a data length check in vgic_its_save_* Message-ID: References: <20240620130650.1279-1-jiangkunkun@huawei.com> <20240620130650.1279-2-jiangkunkun@huawei.com> <50f18621-96a4-cf6e-9fc0-a4e69f77054c@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <50f18621-96a4-cf6e-9fc0-a4e69f77054c@huawei.com> X-Migadu-Flow: FLOW_OUT X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240620_233933_880835_E691B390 X-CRM114-Status: GOOD ( 19.54 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Jun 21, 2024 at 09:43:05AM +0800, Kunkun Jiang wrote: > > static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz) > > { > >         struct its_collection *collection; > >         struct kvm *kvm = its->dev->kvm; > >         u32 target_addr, coll_id; > >         u64 val; > >         int ret; > > > >         BUG_ON(esz > sizeof(val)); > >         ret = kvm_read_guest_lock(kvm, gpa, &val, esz); > It should also be modified synchronously, right? Do you mean replacing the other BUG_ON()'s in the ITS code with the pattern I'd recommended earlier? That'd be great if you could do that. > > > return vgic_write_guest_lock(kvm, gpa, &val, ite_esz); > > > } > > > @@ -2246,6 +2247,7 @@ static int vgic_its_save_dte(struct vgic_its *its, struct its_device *dev, > > > (itt_addr_field << KVM_ITS_DTE_ITTADDR_SHIFT) | > > > (dev->num_eventid_bits - 1)); > > > val = cpu_to_le64(val); > > > + BUG_ON(dte_esz > sizeof(dte_esz)); > > Did you even test this? A bit of substitution arrives at: > > > > BUG_ON(8 > sizeof(unsigned int)); > > > > See the issue? > > > > Please do not test these sort of untested patches on the list, it is a > > waste of everyone's time. > Sorry, there is a problem here. I will pay attention to it in the future. > Thank you for your correction. Apologies for being firm on this, but outside of RFCs the expectation is that the author test changes before posting to the list. In that spirit, a reproducer for the issue you observe in KVM selftests would be great. We have some minimal support for dealing with an ITS over there now. -- Thanks, Oliver