linux-kernel - Re: [PATCH v11 12/26] gunyah: vm_mgr: Add/remove user memory regions

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 11 Apr 2023 22:19:50 +0100
From:   Will Deacon <will@...nel.org>
To:     Elliot Berman <quic_eberman@...cinc.com>
Cc:     Alex Elder <elder@...aro.org>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Prakruthi Deepak Heragu <quic_pheragu@...cinc.com>,
        Murali Nalajala <quic_mnalajal@...cinc.com>,
        Trilok Soni <quic_tsoni@...cinc.com>,
        Srivatsa Vaddagiri <quic_svaddagi@...cinc.com>,
        Carl van Schaik <quic_cvanscha@...cinc.com>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Jonathan Corbet <corbet@....net>,
        Bagas Sanjaya <bagasdotme@...il.com>,
        Andy Gross <agross@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Jassi Brar <jassisinghbrar@...il.com>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v11 12/26] gunyah: vm_mgr: Add/remove user memory regions

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