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=-3.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 74748C433E7 for ; Sat, 10 Oct 2020 22:55:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2F3AE20795 for ; Sat, 10 Oct 2020 22:55:30 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="K43vYuvF" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2389209AbgJJWz2 (ORCPT ); Sat, 10 Oct 2020 18:55:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37312 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731431AbgJJTVv (ORCPT ); Sat, 10 Oct 2020 15:21:51 -0400 Received: from mail-ej1-x643.google.com (mail-ej1-x643.google.com [IPv6:2a00:1450:4864:20::643]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B6164C08EA7F for ; Sat, 10 Oct 2020 10:31:13 -0700 (PDT) Received: by mail-ej1-x643.google.com with SMTP id p15so17567186ejm.7 for ; Sat, 10 Oct 2020 10:31:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xsOZOc5wUkDsyS+q4XR7sRY+/Nw08dWS4rH9PR3SdYM=; b=K43vYuvFf4p0HKwpNNYM8EVjuArTRuuNWIw2q4YLC5jyws5WqABCqrRlB8L6Kw/ga/ 9pb07pav9vu70DJL9+qIG+8WZUav68UhLbRS2hY3VnFfDpqam6GcqIBnDSm/T02riaI0 bK3Q77l+IKqRqvdVTvJ6gEi/4zr+9omw8Jdkw= 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=xsOZOc5wUkDsyS+q4XR7sRY+/Nw08dWS4rH9PR3SdYM=; b=gvMfm0wF/HiaJzTl+gqo5RpmTjdVlJYd+A475p1yFbZPU4U1Ip3F9YBnHc5HqAPox7 0lNGyLgqGmN20MzU7fS3D0rqPqDNC6IxL0yvMuCvRP/2fMlKB6x3D9XdjMfW6uQC7aLH 6RJWHrP/54rnKyXMAz0h0QTUZrtJKUfYW2tVFZj8ujynjA+0a47jJE+2pwsnnrdIcL8K Vehd3zt0ofPz7JjgxmDD1+TqQOoW/cGRb3CmMW10Fb3hkFu0fA4jHTa6lmfYMqKl2UDk 64Hm1yqPxzPpZWOr2BQR3ArYLH1Fqs2ISBgRsO97WQDd70t8udKI9le9tRKpxaYY+NFy cxEw== X-Gm-Message-State: AOAM531Ya9/MMXkGxpdz6JeMlqyVBKaoB43t6KVYShS+sM95jOV7wJ+o OJfvObURVJi6c7npZQ+c38Sn+mL3D1tATw== X-Google-Smtp-Source: ABdhPJwKXhLdQGeLYmAXsYirqNNKfvdzrAnAa/6Tyo7zQL+qO9j55lUFiG55WWG5BPHICGRcNvj1gg== X-Received: by 2002:a17:906:f11a:: with SMTP id gv26mr19702198ejb.13.1602351071664; Sat, 10 Oct 2020 10:31:11 -0700 (PDT) Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com. [209.85.128.44]) by smtp.gmail.com with ESMTPSA id b25sm7978387eds.66.2020.10.10.10.31.10 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 10 Oct 2020 10:31:10 -0700 (PDT) Received: by mail-wm1-f44.google.com with SMTP id p15so12875504wmi.4 for ; Sat, 10 Oct 2020 10:31:10 -0700 (PDT) X-Received: by 2002:a1c:8057:: with SMTP id b84mr3433695wmd.116.1602351069766; Sat, 10 Oct 2020 10:31:09 -0700 (PDT) MIME-Version: 1.0 References: <20201009075934.3509076-1-daniel.vetter@ffwll.ch> <20201009075934.3509076-10-daniel.vetter@ffwll.ch> <20201009123421.67a80d72@coco.lan> <20201009122111.GN5177@ziepe.ca> <20201009143723.45609bfb@coco.lan> In-Reply-To: <20201009143723.45609bfb@coco.lan> From: Tomasz Figa Date: Sat, 10 Oct 2020 19:30:59 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn To: Mauro Carvalho Chehab Cc: Jason Gunthorpe , Daniel Vetter , DRI Development , LKML , kvm , Linux MM , "list@263.net:IOMMU DRIVERS , Joerg Roedel ," , linux-samsung-soc , Linux Media Mailing List , linux-s390 , Daniel Vetter , Kees Cook , Dan Williams , Andrew Morton , John Hubbard , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , Jan Kara , Linus Torvalds Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Mauro, On Fri, Oct 9, 2020 at 2:37 PM Mauro Carvalho Chehab wrote: > > Em Fri, 9 Oct 2020 09:21:11 -0300 > Jason Gunthorpe escreveu: > > > On Fri, Oct 09, 2020 at 12:34:21PM +0200, Mauro Carvalho Chehab wrote: > > > Hi, > > > > > > Em Fri, 9 Oct 2020 09:59:26 +0200 > > > Daniel Vetter escreveu: > > > > > > > Way back it was a reasonable assumptions that iomem mappings never > > > > change the pfn range they point at. But this has changed: > > > > > > > > - gpu drivers dynamically manage their memory nowadays, invalidating > > > > ptes with unmap_mapping_range when buffers get moved > > > > > > > > - contiguous dma allocations have moved from dedicated carvetouts to > > > > cma regions. This means if we miss the unmap the pfn might contain > > > > pagecache or anon memory (well anything allocated with GFP_MOVEABLE) > > > > > > > > - even /dev/mem now invalidates mappings when the kernel requests that > > > > iomem region when CONFIG_IO_STRICT_DEVMEM is set, see 3234ac664a87 > > > > ("/dev/mem: Revoke mappings when a driver claims the region") > > > > > > > > Accessing pfns obtained from ptes without holding all the locks is > > > > therefore no longer a good idea. > > > > > > > > Unfortunately there's some users where this is not fixable (like v4l > > > > userptr of iomem mappings) or involves a pile of work (vfio type1 > > > > iommu). For now annotate these as unsafe and splat appropriately. > > > > > > > > This patch adds an unsafe_follow_pfn, which later patches will then > > > > roll out to all appropriate places. > > > > > > NACK, as this breaks an existing userspace API on media. > > > > It doesn't break it. You get a big warning the thing is broken and it > > keeps working. > > > > We can't leave such a huge security hole open - it impacts other > > subsystems, distros need to be able to run in a secure mode. > > Well, if distros disable it, then apps will break. > Do we have any information on userspace that actually needs this functionality? Note that we're _not_ talking here about the complete USERPTR functionality, but rather just the very corner case of carveout memory not backed by struct pages. Given that the current in-tree ways of reserving carveout memory, such as shared-dma-pool, actually give memory backed by struct pages, do we even have a source of such legacy memory in the kernel today? I think that given that this is a very niche functionality, we could have it disabled by default for security reasons and if someone _really_ (i.e. there is no replacement) needs it, they probably need to use a custom kernel build anyway for their exotic hardware setup (with PFN-backed carveout memory), so they can enable it. > > > While I agree that using the userptr on media is something that > > > new drivers may not support, as DMABUF is a better way of > > > handling it, changing this for existing ones is a big no, > > > as it may break usersapace. > > > > media community needs to work to fix this, not pretend it is OK to > > keep going as-is. > > > Dealing with security issues is the one case where an uABI break might > > be acceptable. > > > > If you want to NAK it then you need to come up with the work to do > > something here correctly that will support the old drivers without the > > kernel taint. > > > > Unfortunately making things uncomfortable for the subsystem is the big > > hammer the core kernel needs to use to actually get this security work > > done by those responsible. > > > I'm not pretending that this is ok. Just pointing that the approach > taken is NOT OK. > > I'm not a mm/ expert, but, from what I understood from Daniel's patch > description is that this is unsafe *only if* __GFP_MOVABLE is used. > > Well, no drivers inside the media subsystem uses such flag, although > they may rely on some infrastructure that could be using it behind > the bars. > > If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE > flag that it would be denying the core mm code to set __GFP_MOVABLE. > > Please let address the issue on this way, instead of broken an > userspace API that it is there since 1991. Note that USERPTR as a whole generally has been considered deprecated in V4L2 for many years and people have been actively discouraged to use it. And, still, we're just talking here about the very rare corner case, not the whole USERPTR API. Best regards, Tomasz 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=-3.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 5EE3FC433DF for ; Sat, 10 Oct 2020 17:33:43 +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 0628B22460 for ; Sat, 10 Oct 2020 17:33:42 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="CaWVJ+bg"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="K43vYuvF" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0628B22460 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org 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=505Rtt8SCYpJ78RHDeh1+gZCsUEabNXeW3HCNTnL1c0=; b=CaWVJ+bgzWYRRsSCQ7V06sqnC S56mzBSTU8u6yFzwWuZEJQNCysK1q3rTa5/uvuWzllApQGp8nA/TTMn+vuz4+jJuDwKcMoCkzKFNp VV8QXRIMGQwf+TQnFDlgWAcGxNJxRm1K+6XHccsbsUeRMHwUtE7tZSGXQC+XORHrtrkuv1o8TlagB UhT2cS6SIeV6WoMtnSbC0XHMlB8dahO6qufD7FHAxYxRXQPHuzkmEWSZLzCuzasejMZ28BuoPbez7 Tla40hNAEt9ifmY+YVgYwm/sl727BmkrREMAvGtk31BPYHa2U1z2eff6OZ/0weW0+MMwajFM5kSIn iTYhTMEAw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kRIiD-0004kz-3X; Sat, 10 Oct 2020 17:31:17 +0000 Received: from mail-ed1-x542.google.com ([2a00:1450:4864:20::542]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kRIi9-0004kQ-SR for linux-arm-kernel@lists.infradead.org; Sat, 10 Oct 2020 17:31:14 +0000 Received: by mail-ed1-x542.google.com with SMTP id 33so12624757edq.13 for ; Sat, 10 Oct 2020 10:31:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xsOZOc5wUkDsyS+q4XR7sRY+/Nw08dWS4rH9PR3SdYM=; b=K43vYuvFf4p0HKwpNNYM8EVjuArTRuuNWIw2q4YLC5jyws5WqABCqrRlB8L6Kw/ga/ 9pb07pav9vu70DJL9+qIG+8WZUav68UhLbRS2hY3VnFfDpqam6GcqIBnDSm/T02riaI0 bK3Q77l+IKqRqvdVTvJ6gEi/4zr+9omw8Jdkw= 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=xsOZOc5wUkDsyS+q4XR7sRY+/Nw08dWS4rH9PR3SdYM=; b=mUtbs+m/6/K/+APbYtd3pqEwmWYU2OOJuNJIImtrFZFPotMVnB1kcClx7chMN8mN9O cVP446zG3defOfFP8b458UWAEOTPOPfjLi+SL1i3PG68ozMiG3/50wPdmP2x9/c+vf62 uL41Z4FqBcSBX7w1SqaX1qAMH0yP6gM6tqe+Mvo4r3ZTgL+RMa2o1vfQD47j/+rhOCs5 dzwFk9BONz+p3aHnBhoZ4t2jznhkQjejj3ccIxrCl0zogF55kwi7b6PCiFdXgRsRefjb NZA76ckBx5PWr5pbLLdmWyQPKXT29QnGkBpQ3GAsWUA3UQtRQrgYpTrjihJcWuT3hWVz U1yw== X-Gm-Message-State: AOAM531M9DkeqR3q4QMypx+1J/fkJLjaE82ze1S3fjVxUwD+3FijJxvE Sd4k5SB7o0ncdx9PWprgkn73/HaLayteDQ== X-Google-Smtp-Source: ABdhPJwGEVQAvYqq48cPP/HTTsaf7u7GbJKaoEcZloAdv7XrQgivEgPxY26HIWOM+bt59W439h4nnQ== X-Received: by 2002:aa7:dada:: with SMTP id x26mr5532019eds.167.1602351071757; Sat, 10 Oct 2020 10:31:11 -0700 (PDT) Received: from mail-wm1-f53.google.com (mail-wm1-f53.google.com. [209.85.128.53]) by smtp.gmail.com with ESMTPSA id dm8sm8046234edb.57.2020.10.10.10.31.10 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 10 Oct 2020 10:31:10 -0700 (PDT) Received: by mail-wm1-f53.google.com with SMTP id d3so12856519wma.4 for ; Sat, 10 Oct 2020 10:31:10 -0700 (PDT) X-Received: by 2002:a1c:8057:: with SMTP id b84mr3433695wmd.116.1602351069766; Sat, 10 Oct 2020 10:31:09 -0700 (PDT) MIME-Version: 1.0 References: <20201009075934.3509076-1-daniel.vetter@ffwll.ch> <20201009075934.3509076-10-daniel.vetter@ffwll.ch> <20201009123421.67a80d72@coco.lan> <20201009122111.GN5177@ziepe.ca> <20201009143723.45609bfb@coco.lan> In-Reply-To: <20201009143723.45609bfb@coco.lan> From: Tomasz Figa Date: Sat, 10 Oct 2020 19:30:59 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn To: Mauro Carvalho Chehab X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201010_133114_006380_15F50F3B X-CRM114-Status: GOOD ( 43.24 ) 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-s390 , linux-samsung-soc , Jan Kara , Kees Cook , kvm , Jason Gunthorpe , Daniel Vetter , LKML , DRI Development , Linux MM , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , John Hubbard , Daniel Vetter , Dan Williams , Linus Torvalds , Andrew Morton , "list@263.net:IOMMU DRIVERS , Joerg Roedel , " , Linux Media Mailing List 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 Hi Mauro, On Fri, Oct 9, 2020 at 2:37 PM Mauro Carvalho Chehab wrote: > > Em Fri, 9 Oct 2020 09:21:11 -0300 > Jason Gunthorpe escreveu: > > > On Fri, Oct 09, 2020 at 12:34:21PM +0200, Mauro Carvalho Chehab wrote: > > > Hi, > > > > > > Em Fri, 9 Oct 2020 09:59:26 +0200 > > > Daniel Vetter escreveu: > > > > > > > Way back it was a reasonable assumptions that iomem mappings never > > > > change the pfn range they point at. But this has changed: > > > > > > > > - gpu drivers dynamically manage their memory nowadays, invalidating > > > > ptes with unmap_mapping_range when buffers get moved > > > > > > > > - contiguous dma allocations have moved from dedicated carvetouts to > > > > cma regions. This means if we miss the unmap the pfn might contain > > > > pagecache or anon memory (well anything allocated with GFP_MOVEABLE) > > > > > > > > - even /dev/mem now invalidates mappings when the kernel requests that > > > > iomem region when CONFIG_IO_STRICT_DEVMEM is set, see 3234ac664a87 > > > > ("/dev/mem: Revoke mappings when a driver claims the region") > > > > > > > > Accessing pfns obtained from ptes without holding all the locks is > > > > therefore no longer a good idea. > > > > > > > > Unfortunately there's some users where this is not fixable (like v4l > > > > userptr of iomem mappings) or involves a pile of work (vfio type1 > > > > iommu). For now annotate these as unsafe and splat appropriately. > > > > > > > > This patch adds an unsafe_follow_pfn, which later patches will then > > > > roll out to all appropriate places. > > > > > > NACK, as this breaks an existing userspace API on media. > > > > It doesn't break it. You get a big warning the thing is broken and it > > keeps working. > > > > We can't leave such a huge security hole open - it impacts other > > subsystems, distros need to be able to run in a secure mode. > > Well, if distros disable it, then apps will break. > Do we have any information on userspace that actually needs this functionality? Note that we're _not_ talking here about the complete USERPTR functionality, but rather just the very corner case of carveout memory not backed by struct pages. Given that the current in-tree ways of reserving carveout memory, such as shared-dma-pool, actually give memory backed by struct pages, do we even have a source of such legacy memory in the kernel today? I think that given that this is a very niche functionality, we could have it disabled by default for security reasons and if someone _really_ (i.e. there is no replacement) needs it, they probably need to use a custom kernel build anyway for their exotic hardware setup (with PFN-backed carveout memory), so they can enable it. > > > While I agree that using the userptr on media is something that > > > new drivers may not support, as DMABUF is a better way of > > > handling it, changing this for existing ones is a big no, > > > as it may break usersapace. > > > > media community needs to work to fix this, not pretend it is OK to > > keep going as-is. > > > Dealing with security issues is the one case where an uABI break might > > be acceptable. > > > > If you want to NAK it then you need to come up with the work to do > > something here correctly that will support the old drivers without the > > kernel taint. > > > > Unfortunately making things uncomfortable for the subsystem is the big > > hammer the core kernel needs to use to actually get this security work > > done by those responsible. > > > I'm not pretending that this is ok. Just pointing that the approach > taken is NOT OK. > > I'm not a mm/ expert, but, from what I understood from Daniel's patch > description is that this is unsafe *only if* __GFP_MOVABLE is used. > > Well, no drivers inside the media subsystem uses such flag, although > they may rely on some infrastructure that could be using it behind > the bars. > > If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE > flag that it would be denying the core mm code to set __GFP_MOVABLE. > > Please let address the issue on this way, instead of broken an > userspace API that it is there since 1991. Note that USERPTR as a whole generally has been considered deprecated in V4L2 for many years and people have been actively discouraged to use it. And, still, we're just talking here about the very rare corner case, not the whole USERPTR API. Best regards, Tomasz _______________________________________________ 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=-3.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no 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 681E3C433E7 for ; Sat, 10 Oct 2020 17:36:32 +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 DA8A22245F for ; Sat, 10 Oct 2020 17:36:31 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="K43vYuvF" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DA8A22245F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org 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 2F18F6F39A; Sat, 10 Oct 2020 17:36:31 +0000 (UTC) Received: from mail-ej1-x642.google.com (mail-ej1-x642.google.com [IPv6:2a00:1450:4864:20::642]) by gabe.freedesktop.org (Postfix) with ESMTPS id 14DDC6F39A for ; Sat, 10 Oct 2020 17:36:30 +0000 (UTC) Received: by mail-ej1-x642.google.com with SMTP id p15so17577707ejm.7 for ; Sat, 10 Oct 2020 10:36:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xsOZOc5wUkDsyS+q4XR7sRY+/Nw08dWS4rH9PR3SdYM=; b=K43vYuvFf4p0HKwpNNYM8EVjuArTRuuNWIw2q4YLC5jyws5WqABCqrRlB8L6Kw/ga/ 9pb07pav9vu70DJL9+qIG+8WZUav68UhLbRS2hY3VnFfDpqam6GcqIBnDSm/T02riaI0 bK3Q77l+IKqRqvdVTvJ6gEi/4zr+9omw8Jdkw= 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=xsOZOc5wUkDsyS+q4XR7sRY+/Nw08dWS4rH9PR3SdYM=; b=eAZ+lCnYZLyqRV7pDuHj1WdHcUp3iIfZpmIDbLLYgIeE2Z6+uNhE1hLzDE9T8ZB7fw hhCD/fj5n/jalBqPDgZgyJnyvcqZujsylbXKPqUEMjUxexV9okopTjNhePYTXMu5nVEL 3UogXPAfeepgVAvTKJ/tsrgxZeLErOvl0vf+35hHGDt/hgjPKxVtFsyLhjuWvYrWTu8B ZAGB08oXpjyDbsaP7u3dAUbmX5VXtAeBZC8oKadUWPQvM1Vy8DuJIxWZbjzpgVOSJFF1 V8ndSJ8HdQIjyGYfDgkI9g0K8jSh1O/tdNqxRxFjmSwQWluZsW8vtUlAFeStd3GFx3nu PT5w== X-Gm-Message-State: AOAM533V2DJCnu+cvvduYTbb7G0GWpQ+hAEY+yJ65XQi3NyLDFxdI/K7 Ljsgj8lFzKLPRzXfbn6UlbevvDwO1o6TRA== X-Google-Smtp-Source: ABdhPJyxTy7ic+fjSrai69a6vfCKyTJxsVv9MmWANpu9XZUU7BYSplA3Q0z8QmHa0gjAHwzDt1iQ5w== X-Received: by 2002:a17:906:b218:: with SMTP id p24mr20114568ejz.136.1602351388253; Sat, 10 Oct 2020 10:36:28 -0700 (PDT) Received: from mail-wm1-f52.google.com (mail-wm1-f52.google.com. [209.85.128.52]) by smtp.gmail.com with ESMTPSA id k9sm7507858ejv.113.2020.10.10.10.36.27 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 10 Oct 2020 10:36:27 -0700 (PDT) Received: by mail-wm1-f52.google.com with SMTP id f21so12900256wml.3 for ; Sat, 10 Oct 2020 10:36:27 -0700 (PDT) X-Received: by 2002:a1c:8057:: with SMTP id b84mr3433695wmd.116.1602351069766; Sat, 10 Oct 2020 10:31:09 -0700 (PDT) MIME-Version: 1.0 References: <20201009075934.3509076-1-daniel.vetter@ffwll.ch> <20201009075934.3509076-10-daniel.vetter@ffwll.ch> <20201009123421.67a80d72@coco.lan> <20201009122111.GN5177@ziepe.ca> <20201009143723.45609bfb@coco.lan> In-Reply-To: <20201009143723.45609bfb@coco.lan> From: Tomasz Figa Date: Sat, 10 Oct 2020 19:30:59 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn To: Mauro Carvalho Chehab 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-s390 , linux-samsung-soc , Jan Kara , Kees Cook , kvm , Jason Gunthorpe , Daniel Vetter , LKML , DRI Development , Linux MM , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , John Hubbard , Daniel Vetter , Dan Williams , Linus Torvalds , Andrew Morton , "list@263.net:IOMMU DRIVERS , Joerg Roedel , " , Linux Media Mailing List Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Mauro, On Fri, Oct 9, 2020 at 2:37 PM Mauro Carvalho Chehab wrote: > > Em Fri, 9 Oct 2020 09:21:11 -0300 > Jason Gunthorpe escreveu: > > > On Fri, Oct 09, 2020 at 12:34:21PM +0200, Mauro Carvalho Chehab wrote: > > > Hi, > > > > > > Em Fri, 9 Oct 2020 09:59:26 +0200 > > > Daniel Vetter escreveu: > > > > > > > Way back it was a reasonable assumptions that iomem mappings never > > > > change the pfn range they point at. But this has changed: > > > > > > > > - gpu drivers dynamically manage their memory nowadays, invalidating > > > > ptes with unmap_mapping_range when buffers get moved > > > > > > > > - contiguous dma allocations have moved from dedicated carvetouts to > > > > cma regions. This means if we miss the unmap the pfn might contain > > > > pagecache or anon memory (well anything allocated with GFP_MOVEABLE) > > > > > > > > - even /dev/mem now invalidates mappings when the kernel requests that > > > > iomem region when CONFIG_IO_STRICT_DEVMEM is set, see 3234ac664a87 > > > > ("/dev/mem: Revoke mappings when a driver claims the region") > > > > > > > > Accessing pfns obtained from ptes without holding all the locks is > > > > therefore no longer a good idea. > > > > > > > > Unfortunately there's some users where this is not fixable (like v4l > > > > userptr of iomem mappings) or involves a pile of work (vfio type1 > > > > iommu). For now annotate these as unsafe and splat appropriately. > > > > > > > > This patch adds an unsafe_follow_pfn, which later patches will then > > > > roll out to all appropriate places. > > > > > > NACK, as this breaks an existing userspace API on media. > > > > It doesn't break it. You get a big warning the thing is broken and it > > keeps working. > > > > We can't leave such a huge security hole open - it impacts other > > subsystems, distros need to be able to run in a secure mode. > > Well, if distros disable it, then apps will break. > Do we have any information on userspace that actually needs this functionality? Note that we're _not_ talking here about the complete USERPTR functionality, but rather just the very corner case of carveout memory not backed by struct pages. Given that the current in-tree ways of reserving carveout memory, such as shared-dma-pool, actually give memory backed by struct pages, do we even have a source of such legacy memory in the kernel today? I think that given that this is a very niche functionality, we could have it disabled by default for security reasons and if someone _really_ (i.e. there is no replacement) needs it, they probably need to use a custom kernel build anyway for their exotic hardware setup (with PFN-backed carveout memory), so they can enable it. > > > While I agree that using the userptr on media is something that > > > new drivers may not support, as DMABUF is a better way of > > > handling it, changing this for existing ones is a big no, > > > as it may break usersapace. > > > > media community needs to work to fix this, not pretend it is OK to > > keep going as-is. > > > Dealing with security issues is the one case where an uABI break might > > be acceptable. > > > > If you want to NAK it then you need to come up with the work to do > > something here correctly that will support the old drivers without the > > kernel taint. > > > > Unfortunately making things uncomfortable for the subsystem is the big > > hammer the core kernel needs to use to actually get this security work > > done by those responsible. > > > I'm not pretending that this is ok. Just pointing that the approach > taken is NOT OK. > > I'm not a mm/ expert, but, from what I understood from Daniel's patch > description is that this is unsafe *only if* __GFP_MOVABLE is used. > > Well, no drivers inside the media subsystem uses such flag, although > they may rely on some infrastructure that could be using it behind > the bars. > > If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE > flag that it would be denying the core mm code to set __GFP_MOVABLE. > > Please let address the issue on this way, instead of broken an > userspace API that it is there since 1991. Note that USERPTR as a whole generally has been considered deprecated in V4L2 for many years and people have been actively discouraged to use it. And, still, we're just talking here about the very rare corner case, not the whole USERPTR API. Best regards, Tomasz _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel