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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4162DC77B75 for ; Tue, 11 Apr 2023 21:20:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229895AbjDKVUb (ORCPT ); Tue, 11 Apr 2023 17:20:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43220 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229844AbjDKVU2 (ORCPT ); Tue, 11 Apr 2023 17:20:28 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A66815FC0; Tue, 11 Apr 2023 14:19:59 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 98812628A3; Tue, 11 Apr 2023 21:19:58 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0C98AC433EF; Tue, 11 Apr 2023 21:19:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1681247998; bh=xYmomJOIpStQiY1yV+hCngqUM+Bx3dX+DeJaga9RHmM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=rhhu/Cl5XMiHoIBrJ8flvp9biCJEEw+bFAY2Tfrd6WKtoMCdaN0XM7kf9Zddkvx9g tOhd7zFD9dCfc+yOgSTxGks9FD5fdRn4gI24ko35zZGBMYVR43mu1kGcLI7fXEW4ah edFiJ1xmHJy8zdo0CsDEX477PFOd1xenQmWk9A3dlASHOqDMdJ15IlGekOPGdV2U6L GJt/+3xk8fqS/5CRpLc1agfzRWz+A+Vhd2+SicC88f6ropLraRixcdT8WE5XKXrmfp j6yxmMv+s+6l70bbJVKEvoIBWQ8WzMPenMBdeWi8IbrjMlrqNnvrhSYgk2Ymp7A9OF 98rp+Z0eYgDKg== Date: Tue, 11 Apr 2023 22:19:50 +0100 From: Will Deacon To: Elliot Berman Cc: Alex Elder , Srinivas Kandagatla , Prakruthi Deepak Heragu , Murali Nalajala , Trilok Soni , Srivatsa Vaddagiri , Carl van Schaik , Dmitry Baryshkov , Bjorn Andersson , Konrad Dybcio , Arnd Bergmann , Greg Kroah-Hartman , Rob Herring , Krzysztof Kozlowski , Jonathan Corbet , Bagas Sanjaya , Andy Gross , Catalin Marinas , Jassi Brar , linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v11 12/26] gunyah: vm_mgr: Add/remove user memory regions Message-ID: <20230411211940.GC23890@willie-the-truck> References: <20230304010632.2127470-1-quic_eberman@quicinc.com> <20230304010632.2127470-13-quic_eberman@quicinc.com> <20230324183659.GB28266@willie-the-truck> <5d1c6160-6bc4-5246-2a0b-de5ddcbbc2c4@quicinc.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5d1c6160-6bc4-5246-2a0b-de5ddcbbc2c4@quicinc.com> User-Agent: Mutt/1.10.1 (2018-07-13) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Apr 11, 2023 at 01:34:34PM -0700, Elliot Berman wrote: > On 3/24/2023 11:37 AM, Will Deacon wrote: > > On Fri, Mar 03, 2023 at 05:06:18PM -0800, Elliot Berman wrote: > > > + > > > + pinned = pin_user_pages_fast(region->userspace_addr, mapping->npages, > > > + FOLL_WRITE | FOLL_LONGTERM, mapping->pages); > > > + if (pinned < 0) { > > > + ret = pinned; > > > + mapping->npages = 0; /* update npages for reclaim */ > > > + goto reclaim; > > > + } else if (pinned != mapping->npages) { > > > + ret = -EFAULT; > > > + mapping->npages = pinned; /* update npages for reclaim */ > > > + goto reclaim; > > > + } > > > > I think Fuad mentioned this on an older version of these patches, but it > > looks like you're failing to account for the pinned memory here which is > > a security issue depending on who is able to issue the ioctl() calling > > into here. > > > > Specifically, I'm thinking that your kXalloc() calls should be using > > GFP_KERNEL_ACCOUNT in this function and also that you should be calling > > account_locked_vm() for the pages being pinned. > > > > Added the accounting for the v12. > > > Finally, what happens if userspace passes in a file mapping? > > Userspace will get EBADADDR (-14) back when trying to launch the VM > (pin_user_pages_fast returns this as you might have been expecting). We > haven't yet had any need to support file-backed mappings. Hmm, no, that's actually surprising to me. I'd have thought GUP would happily pin page-cache pages for file mappings, so I'm intrigued as to which FOLL_ flag is causing you to get an error code back. Can you enlighten me on where the failure originates, please? Will