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 B79B7CD4F4C for ; Mon, 9 Sep 2024 07:17:34 +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-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=VftkMC5+mYm3QQ3dWBsLEgURQE/0zJC3ZLx2ECmB1zs=; b=ElD5kRtDy1aBIlI1Yv0VU7l09Y fzoGb6owLT0cAdWaDu1RvNqxvntKnfhPKZ+laF0N0yRVuYvpUkG/VvhUmXfGPwPgAXNGdQFYNlmCt nwrTVjFcBOE0zmmyFbAanzFHAFa52Y6A32TiVgXtiS3xgV9QazRCtdg6krryyg02INmUl+57iv7Bu DQ/por+utG53JE40ynmldT/t7db4MfMKNIoCZ8J+Kmi5H+PRfq4cOqF1/HCQNniveSUX9dCwwos2I VtzaiCfAGhUXRnBC9nBej/lDnW1Bz8KHlt9SD3iErq/9x59Rh8RV6MsLyUdQjybIB5rtDyG/A0wuw YAiqKGAQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1snYeH-00000000mKJ-440K; Mon, 09 Sep 2024 07:17:21 +0000 Received: from mail-wm1-x32d.google.com ([2a00:1450:4864:20::32d]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1snYdH-00000000mC5-04oh for linux-arm-kernel@lists.infradead.org; Mon, 09 Sep 2024 07:16:20 +0000 Received: by mail-wm1-x32d.google.com with SMTP id 5b1f17b1804b1-427fc9834deso58745e9.0 for ; Mon, 09 Sep 2024 00:16:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1725866176; x=1726470976; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=VftkMC5+mYm3QQ3dWBsLEgURQE/0zJC3ZLx2ECmB1zs=; b=vrzQ1Ey1RB1QTQAbmjEMLA/VgDVNyBweawTrRPhIZbG0RNaP3BDISrxwdk2PfJnC3t 2v5JhO6SX55saIvaWuTn8sEnPuNGknfF1ZdD3FRwwIbOZjBJ6RWLnIrlantmm+l2vjBO E/7s3XKyWtgXBQOqwEChgGwwxtS0BS4SVka+BucjrPZuDQTEYfy06aZM6ebbGsyddS7s 3V1ovmq17EQ2/7Y7Hk7A0wld90BBA5jEm7emi5YY0ggDLakWiuPHEyPeQyXLayMGM97m 0WXg75TtT2WcEK4m0fRw5uYIozav8EbX6MJaDG3sWmT4kJOrGgjyjlnvjRoSz6kpk3Rb A6FA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725866176; x=1726470976; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=VftkMC5+mYm3QQ3dWBsLEgURQE/0zJC3ZLx2ECmB1zs=; b=EXYk50rUQa2Y5TuvBuBW9zhKFhmEP+zTPxQqVFzX8tnpg40T0uE3s6WvfWVGTKNFP+ 4KXDR3u1kbznGngrWldgCRlDdequzw70DDuhEIIcIKfrpTPeXc842KqFQINVPL4wNhIF EwZfYzR8Kxpg1a6iqv1XSjCxEO23/P54l6HwwMEsN9WgvYrFp/1f3stnxZ6WvFm3FLcf 0yG7XVyRDujtDpB23eTeJ9mRUbs14sDlT7DRQEHnMSbENCMz0+PLw6LbpPFl+DltDXz8 rVSnbaMmNinYhwveRE6wI2z0MssIT0NsmdlXdsZxYT2pHEo14Ue7LYnSbsjVgfSDQlng qEcQ== X-Forwarded-Encrypted: i=1; AJvYcCUVRPlg4qjJ1EmI9mfjvovPswXJoBaQkLMs/87L7BUj6OdxHr0yLxV/iJBwz+OFYoqSuqHpFLOVgmXwLIC5GBW2@lists.infradead.org X-Gm-Message-State: AOJu0YzTPxPq8RgEsdFbbJZ/XOkN9ZLgjJ7K388NkGFvFFtHStEsF5z+ ip6iuTTEURG0jTv95575r4lcGkF8xz1AES27XOzllZuX+faT+5jDNyaeAMfPcQ== X-Google-Smtp-Source: AGHT+IGOIkTSaWnXjE2bwYgTX4o/x860SYJOr8Q/IVHIUjLL5fLSHy00YIBudEH8NGk3Gpb6Mxcv4g== X-Received: by 2002:a05:600c:3d17:b0:426:68ce:c97a with SMTP id 5b1f17b1804b1-42cae4bc174mr1825485e9.7.1725866175872; Mon, 09 Sep 2024 00:16:15 -0700 (PDT) Received: from google.com (93.20.140.34.bc.googleusercontent.com. [34.140.20.93]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-42caf33e9b2sm65999675e9.14.2024.09.09.00.16.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Sep 2024 00:16:15 -0700 (PDT) Date: Mon, 9 Sep 2024 07:16:13 +0000 From: Sebastian Ene To: Marc Zyngier Cc: Snehal Koukuntla , Oliver Upton , James Morse , Suzuki K Poulose , Zenghui Yu , Catalin Marinas , Will Deacon , Sudeep Holla , Vincent Donnefort , Jean-Philippe Brucker , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH] KVM: arm64: Add memory length checks before it is xfered Message-ID: References: <20240906092732.113152-1-snehalreddy@google.com> <86ed5wvixw.wl-maz@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <86ed5wvixw.wl-maz@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240909_001619_109406_2AB7FF63 X-CRM114-Status: GOOD ( 35.17 ) 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, Sep 06, 2024 at 05:35:39PM +0100, Marc Zyngier wrote: Hi, > Hi Snehal, > > On Fri, 06 Sep 2024 10:27:32 +0100, > Snehal Koukuntla wrote: > > > > From: Snehal > > > > Check size during allocation to fix discrepancy in memory reclaim path. > > Currently only happens during memory reclaim, inconsistent with mem_xfer > > Can you please elaborate? It doesn't seem to fail at allocation time > here, as everything is pre-allocated. Some context would greatly help, > as my FFA-foo is as basic as it gets (I did read the spec once and ran > away screaming). > Right, I think what happens is that we use the fragmentation API to transfer memory to Trustzone that normally won't fit on the reclaim path where we use an auxiliary buffer to store the descriptors. All the descriptors are identified by the same handle and the reclaim will try to store them into the ffa_desc_buf before nuking the FF-A annotation from the stage-2. > > > > Signed-off-by: Snehal Koukuntla > > The From: and Signed-off-by: tags do not match. You may want to add a > [user] section to your .gitconfig with your full name so that this > issue is sorted once and for all. > > > --- > > arch/arm64/kvm/hyp/nvhe/ffa.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c > > index e715c157c2c4..e9223cc4f913 100644 > > --- a/arch/arm64/kvm/hyp/nvhe/ffa.c > > +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c > > @@ -461,6 +461,11 @@ static __always_inline void do_ffa_mem_xfer(const u64 func_id, > > /facepalm: why do we have this __always_inline here? Nothing to do > with your patch, but definitely worth understanding why it is > required. > I don't think it is needed, we can drop it. Maybe as part of this patch ? > > goto out_unlock; > > } > > > > + if (len > ffa_desc_buf.len) { > > + ret = FFA_RET_NO_MEMORY; > > + goto out_unlock; > > + } > > + > > It took some digging to understand how the various queues are sized, > and a comment explaining the relation between ffa_desc_buf and the > other queues would be very welcome. > > I also notice that we have other places (apparently dealing with > fragments) that do not have such checks. Do they need anything else? > I think we don't need that check in other parts. > > buf = hyp_buffers.tx; > > memcpy(buf, host_buffers.tx, fraglen); > > > > Finally, this probably deserves a Fixes: tag and a Cc: stable so that > it can be backported. > > Thanks, > > M. > Seb > -- > Without deviation from the norm, progress is not possible.