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 X-Spam-Level: X-Spam-Status: No, score=-9.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6267EC4727F for ; Mon, 5 Oct 2020 18:16:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 075AD2100A for ; Mon, 5 Oct 2020 18:16:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="J0+Lgox+" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727650AbgJESQ0 (ORCPT ); Mon, 5 Oct 2020 14:16:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46484 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727261AbgJESQ0 (ORCPT ); Mon, 5 Oct 2020 14:16:26 -0400 Received: from mail-ot1-x342.google.com (mail-ot1-x342.google.com [IPv6:2607:f8b0:4864:20::342]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BFA3CC0613CE for ; Mon, 5 Oct 2020 11:16:24 -0700 (PDT) Received: by mail-ot1-x342.google.com with SMTP id 60so9553554otw.3 for ; Mon, 05 Oct 2020 11:16:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=o2+egbzisCjaq6nT3TbPZNwblZFYsu8ELcSxPmHmJLs=; b=J0+Lgox+Zl9MR/XcCJjFK6iKg91x1iw1K/lYf26aRsAWa8kYykCWy7QLhvJYCZo/rI 85l/kYnB3V0HE1AMuIcpmFW5+WsQgw3xUFa+j7WIrU1oW0leAypRqp05SrgmM9EUXOaD hCEkIII06FI//qIehI2esTVFpziKPwVbJXmP0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=o2+egbzisCjaq6nT3TbPZNwblZFYsu8ELcSxPmHmJLs=; b=qPCAaasvAh3CUktzPsuNyo1OZt0RkoriRiW4VirGBmr7N0NrKawMgDLrLbNUND7AUP 9veIGnp9O0hfHh8xpqnP0v4X6QQ+nW9yFbDHShXzH8VRjwePn7hp8luGXfbYKQqg6Mzw 1piAd59+Wz2r6nBBl4CLMKUsJmsOpmKCq004G63GvvadyNra+NgVPIB2b4y33jLXWMKA VWC0deL4HxBDNZBRime8RpbZ7+SAJmFKqvtKDYbAwSTbKSG+RVZagLwmvFHAVZo42aPs S46y8BqgEvTpJ+eulJZPXfIM/51mmwa483JqzLiPsF2WEwsrPTLJiVOmzt8BFVlFh/as kOzw== X-Gm-Message-State: AOAM5302pJAm8iMPboD6pseosnybA5eD1x/wSVmQz9a6OyTsKe26xIS6 l91dLjZkn83K5zmvke9xLg0bR2SMOd4pL6akcZkr9A== X-Google-Smtp-Source: ABdhPJw1WdHFszqHjMyHkkFFS0NFpEwjo9cLd/GAkbhzhWZSM2RoYOtYwo36XdXuPtVVYQY/y+MJINm3GzBdV6PWwoE= X-Received: by 2002:a05:6830:1647:: with SMTP id h7mr392769otr.281.1601921784105; Mon, 05 Oct 2020 11:16:24 -0700 (PDT) MIME-Version: 1.0 References: <0-v1-447bb60c11dd+174-frame_vec_fix_jgg@nvidia.com> <20201005175308.GI4225@quack2.suse.cz> <20201005175746.GA4734@nvidia.com> In-Reply-To: <20201005175746.GA4734@nvidia.com> From: Daniel Vetter Date: Mon, 5 Oct 2020 20:16:12 +0200 Message-ID: Subject: Re: [PATCH 2/2] mm/frame-vec: use FOLL_LONGTERM To: Jason Gunthorpe Cc: Jan Kara , andrew Morton , Linux MM , Hans Verkuil , Mauro Carvalho Chehab , Mel Gorman , stable , Vlastimil Babka , John Hubbard , DRI Development , LKML , Dan Williams , Linux ARM , linux-samsung-soc , "open list:DMA BUFFER SHARING FRAMEWORK" Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Oct 5, 2020 at 7:58 PM Jason Gunthorpe wrote: > > On Mon, Oct 05, 2020 at 07:53:08PM +0200, Jan Kara wrote: > > On Mon 05-10-20 14:38:54, Jason Gunthorpe wrote: > > > When get_vaddr_frames() does its hacky follow_pfn() loop it should never > > > be allowed to extract a struct page from a normal VMA. This could allow a > > > serious use-after-free problem on any kernel memory. > > > > > > Restrict this to only work on VMA's with one of VM_IO | VM_PFNMAP > > > set. This limits the use-after-free problem to only IO memory, which while > > > still serious, is an improvement. > > > > > > Cc: stable@vger.kernel.org > > > Fixes: 8025e5ddf9c1 ("[media] mm: Provide new get_vaddr_frames() helper") > > > Signed-off-by: Jason Gunthorpe > > > mm/frame_vector.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/mm/frame_vector.c b/mm/frame_vector.c > > > index 10f82d5643b6de..26cb20544b6c37 100644 > > > +++ b/mm/frame_vector.c > > > @@ -99,6 +99,10 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames, > > > if (ret >= nr_frames || start < vma->vm_end) > > > break; > > > vma = find_vma_intersection(mm, start, start + 1); > > > + if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) { > > > + ret = -EINVAL; > > > + goto out; > > > + } > > > } while (vma && vma->vm_flags & (VM_IO | VM_PFNMAP)); > > > > Hum, I fail to see how this helps. If vma has no VM_IO or VM_PFNMAP flag, > > we'd exit the loop (to out: label) anyway due to the loop termination > > condition and why not return the frames we already have? Furthermore > > find_vma_intersection() can return NULL which would oops in your check > > then. What am I missing? > > Oh, nothing, you are right. It just didn't read naturally because > hitting the wrong kind of VMA should be an error condition :\ Afaik these mmio maps should all be VM_DONTEXPAND (or at least the ones in drivers/gpu are all), so not sure why we need the loop here. But maybe there's some drivers that don't set that, or have other funny things going on with userspace piecing the mmap together, and I'm not going to audit them all :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch 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 X-Spam-Level: X-Spam-Status: No, score=-9.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 741E5C4741F for ; Mon, 5 Oct 2020 18:16:28 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 4A65E20B80 for ; Mon, 5 Oct 2020 18:16:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="J0+Lgox+" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4A65E20B80 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id B51D4900010; Mon, 5 Oct 2020 14:16:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B02D890000C; Mon, 5 Oct 2020 14:16:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9F0A2900010; Mon, 5 Oct 2020 14:16:26 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0042.hostedemail.com [216.40.44.42]) by kanga.kvack.org (Postfix) with ESMTP id 6F37A90000C for ; Mon, 5 Oct 2020 14:16:26 -0400 (EDT) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 0CD46362D for ; Mon, 5 Oct 2020 18:16:26 +0000 (UTC) X-FDA: 77338676772.07.skirt61_2017e56271c0 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin07.hostedemail.com (Postfix) with ESMTP id D33411803F9A2 for ; Mon, 5 Oct 2020 18:16:25 +0000 (UTC) X-HE-Tag: skirt61_2017e56271c0 X-Filterd-Recvd-Size: 5378 Received: from mail-ot1-f67.google.com (mail-ot1-f67.google.com [209.85.210.67]) by imf15.hostedemail.com (Postfix) with ESMTP for ; Mon, 5 Oct 2020 18:16:25 +0000 (UTC) Received: by mail-ot1-f67.google.com with SMTP id i12so3729173ota.5 for ; Mon, 05 Oct 2020 11:16:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=o2+egbzisCjaq6nT3TbPZNwblZFYsu8ELcSxPmHmJLs=; b=J0+Lgox+Zl9MR/XcCJjFK6iKg91x1iw1K/lYf26aRsAWa8kYykCWy7QLhvJYCZo/rI 85l/kYnB3V0HE1AMuIcpmFW5+WsQgw3xUFa+j7WIrU1oW0leAypRqp05SrgmM9EUXOaD hCEkIII06FI//qIehI2esTVFpziKPwVbJXmP0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=o2+egbzisCjaq6nT3TbPZNwblZFYsu8ELcSxPmHmJLs=; b=Z6WBSbVVIbZY9F+5llbVunwqxb/y1vSsySpVBh1OyU0vumRVUli5DXZr7YU82Fsl/F INUbIK2CgaJyTVZDsbwmOXiavOayHXvqDAMCcJTZAoYyXCwU5qnbfYbyAB25SX0KUQNt 4Ff/2ge/RNOVWl2T7kLmLb0a9yKD26Jnwgd9EPqy/hPls5d5EvKGU7NDS9DkR57n6Fq6 GFU/wMJWljKDE8BOASdbSkHVSs7ehwv1InUR+MLlmT9FfI4WSu4b+WsPHxGhYj9VtOkh 7UwYWDs74iw3UxxEgljtEF3p6Qi4orEGG9dsg5Mv9fan0IMQCVyM7cbMsAdgRxujHVIr WH2Q== X-Gm-Message-State: AOAM530VjL0S5zZmp7OS22XUO8zDTMFaenen2Xe7Iuhf50PtQl5rBhTV yZtOSpWoRbbrSGsRmmwvKeXRvZ5KhdNx0S7jSGRwTQ== X-Google-Smtp-Source: ABdhPJw1WdHFszqHjMyHkkFFS0NFpEwjo9cLd/GAkbhzhWZSM2RoYOtYwo36XdXuPtVVYQY/y+MJINm3GzBdV6PWwoE= X-Received: by 2002:a05:6830:1647:: with SMTP id h7mr392769otr.281.1601921784105; Mon, 05 Oct 2020 11:16:24 -0700 (PDT) MIME-Version: 1.0 References: <0-v1-447bb60c11dd+174-frame_vec_fix_jgg@nvidia.com> <20201005175308.GI4225@quack2.suse.cz> <20201005175746.GA4734@nvidia.com> In-Reply-To: <20201005175746.GA4734@nvidia.com> From: Daniel Vetter Date: Mon, 5 Oct 2020 20:16:12 +0200 Message-ID: Subject: Re: [PATCH 2/2] mm/frame-vec: use FOLL_LONGTERM To: Jason Gunthorpe Cc: Jan Kara , andrew Morton , Linux MM , Hans Verkuil , Mauro Carvalho Chehab , Mel Gorman , stable , Vlastimil Babka , John Hubbard , DRI Development , LKML , Dan Williams , Linux ARM , linux-samsung-soc , "open list:DMA BUFFER SHARING FRAMEWORK" Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Oct 5, 2020 at 7:58 PM Jason Gunthorpe wrote: > > On Mon, Oct 05, 2020 at 07:53:08PM +0200, Jan Kara wrote: > > On Mon 05-10-20 14:38:54, Jason Gunthorpe wrote: > > > When get_vaddr_frames() does its hacky follow_pfn() loop it should never > > > be allowed to extract a struct page from a normal VMA. This could allow a > > > serious use-after-free problem on any kernel memory. > > > > > > Restrict this to only work on VMA's with one of VM_IO | VM_PFNMAP > > > set. This limits the use-after-free problem to only IO memory, which while > > > still serious, is an improvement. > > > > > > Cc: stable@vger.kernel.org > > > Fixes: 8025e5ddf9c1 ("[media] mm: Provide new get_vaddr_frames() helper") > > > Signed-off-by: Jason Gunthorpe > > > mm/frame_vector.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/mm/frame_vector.c b/mm/frame_vector.c > > > index 10f82d5643b6de..26cb20544b6c37 100644 > > > +++ b/mm/frame_vector.c > > > @@ -99,6 +99,10 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames, > > > if (ret >= nr_frames || start < vma->vm_end) > > > break; > > > vma = find_vma_intersection(mm, start, start + 1); > > > + if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) { > > > + ret = -EINVAL; > > > + goto out; > > > + } > > > } while (vma && vma->vm_flags & (VM_IO | VM_PFNMAP)); > > > > Hum, I fail to see how this helps. If vma has no VM_IO or VM_PFNMAP flag, > > we'd exit the loop (to out: label) anyway due to the loop termination > > condition and why not return the frames we already have? Furthermore > > find_vma_intersection() can return NULL which would oops in your check > > then. What am I missing? > > Oh, nothing, you are right. It just didn't read naturally because > hitting the wrong kind of VMA should be an error condition :\ Afaik these mmio maps should all be VM_DONTEXPAND (or at least the ones in drivers/gpu are all), so not sure why we need the loop here. But maybe there's some drivers that don't set that, or have other funny things going on with userspace piecing the mmap together, and I'm not going to audit them all :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch 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 X-Spam-Level: X-Spam-Status: No, score=-10.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 71CCCC4363A for ; Mon, 5 Oct 2020 18:18:05 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 17E9320853 for ; Mon, 5 Oct 2020 18:18:05 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="XdZpaaai"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="J0+Lgox+" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 17E9320853 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=sWy67nADgcYzGYDmOb6SsWkr/xndKde6e+P/6IS8Emg=; b=XdZpaaai41ScFrruH5TGT6ytZ KO3bHYhQh40lmOqQTwwFbw/Y97xDwziTPxKDRD/YbNRngihQj2KSlLpYdoPoC3e08ObYBygerDBg3 1m+32KQ1cJxqxIRvVJkctWwuBygZVFh1kAqReq/Zww8ZQ+Gss5sq3ublGd8UIopwY7qLkFPDb0tl8 v5LLz2gPGQdeVeUD9fhrrMizDOyx0msG3mVoxMlQ1Cj0ZHQ+yC9+sqSoPhvYA2MEAcCzuxzvdWPYr 9iBSsZ9+g/5ko91Z61xx72gZVVQkjF89V430goN6CoYpD8YLosr9rn737a7mJB4Wmg66m+JPeHlMC NLEWv3uXg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kPV2E-0007dd-Uo; Mon, 05 Oct 2020 18:16:30 +0000 Received: from mail-ot1-x344.google.com ([2607:f8b0:4864:20::344]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kPV2D-0007cT-1b for linux-arm-kernel@lists.infradead.org; Mon, 05 Oct 2020 18:16:29 +0000 Received: by mail-ot1-x344.google.com with SMTP id m12so9567960otr.0 for ; Mon, 05 Oct 2020 11:16:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=o2+egbzisCjaq6nT3TbPZNwblZFYsu8ELcSxPmHmJLs=; b=J0+Lgox+Zl9MR/XcCJjFK6iKg91x1iw1K/lYf26aRsAWa8kYykCWy7QLhvJYCZo/rI 85l/kYnB3V0HE1AMuIcpmFW5+WsQgw3xUFa+j7WIrU1oW0leAypRqp05SrgmM9EUXOaD hCEkIII06FI//qIehI2esTVFpziKPwVbJXmP0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=o2+egbzisCjaq6nT3TbPZNwblZFYsu8ELcSxPmHmJLs=; b=tHj2R1VUh3QHLn/rlUcjf7y3DJGvmUUGdh729FDiFKfjhfwqL0P6FdBPMrTXok/UnL vfNb+jrAhXs4Ulri/8xrFgPe9DN8hzw++CYF/Cltn8ydQIRG/aSB44r7FLZO2I4MXcY1 O+uoSKQSOqLZ/Z6paCUrZAMleqCzwiQJsfVtp4fe+lKuJ5mLTWQ2rDJFOIXzWqBaB4wF Mm+IueFTuAiKKgoHQAtMqoCqFWJA02jhQzM40di+pkhpvDKQNHmf5NHz5S9oHeWbWyTB KTbMsitwY1QcxJ1EF9lXHv4z4COD77rgflQVJjOhKnSdk87zw6ibKEVXBDhyCg0U7ETz Z6lA== X-Gm-Message-State: AOAM531sE7ffDsZYwFd61muIDy+39O1TPD/4sLkzGDXIxxcPsA/CDdbe 9lnlvw1ZkiAVF7huaZp6cMIZ5E74HCSd30Lik/5Yqg== X-Google-Smtp-Source: ABdhPJw1WdHFszqHjMyHkkFFS0NFpEwjo9cLd/GAkbhzhWZSM2RoYOtYwo36XdXuPtVVYQY/y+MJINm3GzBdV6PWwoE= X-Received: by 2002:a05:6830:1647:: with SMTP id h7mr392769otr.281.1601921784105; Mon, 05 Oct 2020 11:16:24 -0700 (PDT) MIME-Version: 1.0 References: <0-v1-447bb60c11dd+174-frame_vec_fix_jgg@nvidia.com> <20201005175308.GI4225@quack2.suse.cz> <20201005175746.GA4734@nvidia.com> In-Reply-To: <20201005175746.GA4734@nvidia.com> From: Daniel Vetter Date: Mon, 5 Oct 2020 20:16:12 +0200 Message-ID: Subject: Re: [PATCH 2/2] mm/frame-vec: use FOLL_LONGTERM To: Jason Gunthorpe X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201005_141629_097746_950754F6 X-CRM114-Status: GOOD ( 26.85 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Linux ARM , linux-samsung-soc , Jan Kara , Mauro Carvalho Chehab , John Hubbard , DRI Development , LKML , stable , Linux MM , Hans Verkuil , Mel Gorman , andrew Morton , Dan Williams , Vlastimil Babka , "open list:DMA BUFFER SHARING FRAMEWORK" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Oct 5, 2020 at 7:58 PM Jason Gunthorpe wrote: > > On Mon, Oct 05, 2020 at 07:53:08PM +0200, Jan Kara wrote: > > On Mon 05-10-20 14:38:54, Jason Gunthorpe wrote: > > > When get_vaddr_frames() does its hacky follow_pfn() loop it should never > > > be allowed to extract a struct page from a normal VMA. This could allow a > > > serious use-after-free problem on any kernel memory. > > > > > > Restrict this to only work on VMA's with one of VM_IO | VM_PFNMAP > > > set. This limits the use-after-free problem to only IO memory, which while > > > still serious, is an improvement. > > > > > > Cc: stable@vger.kernel.org > > > Fixes: 8025e5ddf9c1 ("[media] mm: Provide new get_vaddr_frames() helper") > > > Signed-off-by: Jason Gunthorpe > > > mm/frame_vector.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/mm/frame_vector.c b/mm/frame_vector.c > > > index 10f82d5643b6de..26cb20544b6c37 100644 > > > +++ b/mm/frame_vector.c > > > @@ -99,6 +99,10 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames, > > > if (ret >= nr_frames || start < vma->vm_end) > > > break; > > > vma = find_vma_intersection(mm, start, start + 1); > > > + if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) { > > > + ret = -EINVAL; > > > + goto out; > > > + } > > > } while (vma && vma->vm_flags & (VM_IO | VM_PFNMAP)); > > > > Hum, I fail to see how this helps. If vma has no VM_IO or VM_PFNMAP flag, > > we'd exit the loop (to out: label) anyway due to the loop termination > > condition and why not return the frames we already have? Furthermore > > find_vma_intersection() can return NULL which would oops in your check > > then. What am I missing? > > Oh, nothing, you are right. It just didn't read naturally because > hitting the wrong kind of VMA should be an error condition :\ Afaik these mmio maps should all be VM_DONTEXPAND (or at least the ones in drivers/gpu are all), so not sure why we need the loop here. But maybe there's some drivers that don't set that, or have other funny things going on with userspace piecing the mmap together, and I'm not going to audit them all :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel 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 X-Spam-Level: X-Spam-Status: No, score=-9.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1DCBAC46466 for ; Mon, 5 Oct 2020 18:16:28 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4D45821481 for ; Mon, 5 Oct 2020 18:16:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="J0+Lgox+" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4D45821481 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4537389C59; Mon, 5 Oct 2020 18:16:26 +0000 (UTC) Received: from mail-ot1-x344.google.com (mail-ot1-x344.google.com [IPv6:2607:f8b0:4864:20::344]) by gabe.freedesktop.org (Postfix) with ESMTPS id E77B589C2C for ; Mon, 5 Oct 2020 18:16:24 +0000 (UTC) Received: by mail-ot1-x344.google.com with SMTP id o8so9550937otl.4 for ; Mon, 05 Oct 2020 11:16:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=o2+egbzisCjaq6nT3TbPZNwblZFYsu8ELcSxPmHmJLs=; b=J0+Lgox+Zl9MR/XcCJjFK6iKg91x1iw1K/lYf26aRsAWa8kYykCWy7QLhvJYCZo/rI 85l/kYnB3V0HE1AMuIcpmFW5+WsQgw3xUFa+j7WIrU1oW0leAypRqp05SrgmM9EUXOaD hCEkIII06FI//qIehI2esTVFpziKPwVbJXmP0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=o2+egbzisCjaq6nT3TbPZNwblZFYsu8ELcSxPmHmJLs=; b=lQDkY74352mc4JukpR91Fhqq50NLCTs4Lru5Yy/XpPu2fhhzGZA6fN+fxoZdD/f4er 7ijIIk0lvUlDHoUyBnEGIdVuHwK20e5V8sIOV+WWS8CluAUU9nxMBpOFJUetfe45/ccj 21tR1gZCxhS3EoPKchh3cC74ocv+KHT8MX1GVkfMVQ86t5HcMl6GYjXVA2UE9MQ3uqUo 8/yhpC+p2eTiixX+StHqwn6eLz040FNFmMo6NfQ+Ws0OU7Ym3D62w/Oe9WU7tlMxY9oz F3XMlmN1xww89xM9l0gsfA/koVd4yo+RvOEm26J8iaAailr6M22GCwyzqC9nVBqson4S uLnw== X-Gm-Message-State: AOAM530lms3N2vz1XnrSBXRTiQNnYt456kbwwzp6JCpXBg6W44f+wdKA vhSAv7SZru25RaKLxfvd+oUTHJwghvLqpHVR/073Qw== X-Google-Smtp-Source: ABdhPJw1WdHFszqHjMyHkkFFS0NFpEwjo9cLd/GAkbhzhWZSM2RoYOtYwo36XdXuPtVVYQY/y+MJINm3GzBdV6PWwoE= X-Received: by 2002:a05:6830:1647:: with SMTP id h7mr392769otr.281.1601921784105; Mon, 05 Oct 2020 11:16:24 -0700 (PDT) MIME-Version: 1.0 References: <0-v1-447bb60c11dd+174-frame_vec_fix_jgg@nvidia.com> <20201005175308.GI4225@quack2.suse.cz> <20201005175746.GA4734@nvidia.com> In-Reply-To: <20201005175746.GA4734@nvidia.com> From: Daniel Vetter Date: Mon, 5 Oct 2020 20:16:12 +0200 Message-ID: Subject: Re: [PATCH 2/2] mm/frame-vec: use FOLL_LONGTERM To: Jason Gunthorpe X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Linux ARM , linux-samsung-soc , Jan Kara , Mauro Carvalho Chehab , John Hubbard , DRI Development , LKML , stable , Linux MM , Hans Verkuil , Mel Gorman , andrew Morton , Dan Williams , Vlastimil Babka , "open list:DMA BUFFER SHARING FRAMEWORK" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Mon, Oct 5, 2020 at 7:58 PM Jason Gunthorpe wrote: > > On Mon, Oct 05, 2020 at 07:53:08PM +0200, Jan Kara wrote: > > On Mon 05-10-20 14:38:54, Jason Gunthorpe wrote: > > > When get_vaddr_frames() does its hacky follow_pfn() loop it should never > > > be allowed to extract a struct page from a normal VMA. This could allow a > > > serious use-after-free problem on any kernel memory. > > > > > > Restrict this to only work on VMA's with one of VM_IO | VM_PFNMAP > > > set. This limits the use-after-free problem to only IO memory, which while > > > still serious, is an improvement. > > > > > > Cc: stable@vger.kernel.org > > > Fixes: 8025e5ddf9c1 ("[media] mm: Provide new get_vaddr_frames() helper") > > > Signed-off-by: Jason Gunthorpe > > > mm/frame_vector.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/mm/frame_vector.c b/mm/frame_vector.c > > > index 10f82d5643b6de..26cb20544b6c37 100644 > > > +++ b/mm/frame_vector.c > > > @@ -99,6 +99,10 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames, > > > if (ret >= nr_frames || start < vma->vm_end) > > > break; > > > vma = find_vma_intersection(mm, start, start + 1); > > > + if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) { > > > + ret = -EINVAL; > > > + goto out; > > > + } > > > } while (vma && vma->vm_flags & (VM_IO | VM_PFNMAP)); > > > > Hum, I fail to see how this helps. If vma has no VM_IO or VM_PFNMAP flag, > > we'd exit the loop (to out: label) anyway due to the loop termination > > condition and why not return the frames we already have? Furthermore > > find_vma_intersection() can return NULL which would oops in your check > > then. What am I missing? > > Oh, nothing, you are right. It just didn't read naturally because > hitting the wrong kind of VMA should be an error condition :\ Afaik these mmio maps should all be VM_DONTEXPAND (or at least the ones in drivers/gpu are all), so not sure why we need the loop here. But maybe there's some drivers that don't set that, or have other funny things going on with userspace piecing the mmap together, and I'm not going to audit them all :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel