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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 219B4C77B60 for ; Sun, 23 Apr 2023 22:29:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 403F96B0071; Sun, 23 Apr 2023 18:29:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3B4026B0074; Sun, 23 Apr 2023 18:29:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 254566B0075; Sun, 23 Apr 2023 18:29:48 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 132D56B0071 for ; Sun, 23 Apr 2023 18:29:48 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id ADE1C40343 for ; Sun, 23 Apr 2023 22:29:47 +0000 (UTC) X-FDA: 80714099214.16.F7DC291 Received: from mail-pl1-f175.google.com (mail-pl1-f175.google.com [209.85.214.175]) by imf04.hostedemail.com (Postfix) with ESMTP id C450540009 for ; Sun, 23 Apr 2023 22:29:45 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=fromorbit-com.20221208.gappssmtp.com header.s=20221208 header.b=CSv4RH6S; spf=pass (imf04.hostedemail.com: domain of david@fromorbit.com designates 209.85.214.175 as permitted sender) smtp.mailfrom=david@fromorbit.com; dmarc=pass (policy=quarantine) header.from=fromorbit.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1682288985; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=WCA3zsna6vFeV798erY3+vSi/BvHgoXGHGGpFGJHBjM=; b=vxd4+c5ZtwcscKYVSK6EIlqTRHCP8G2wpFPnIrocDbx4UvQJrgz7Lsy4f4dZVfoVqY997l aF9ZjtXKKLX+rSHtrozhqljSHADsl92ad/1s8PoSDVvz5BStrWoXZdR+swB1uxr8L7RHBv 05mwft4oUz97s/BqZb40xojTzgNimVI= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=fromorbit-com.20221208.gappssmtp.com header.s=20221208 header.b=CSv4RH6S; spf=pass (imf04.hostedemail.com: domain of david@fromorbit.com designates 209.85.214.175 as permitted sender) smtp.mailfrom=david@fromorbit.com; dmarc=pass (policy=quarantine) header.from=fromorbit.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1682288985; a=rsa-sha256; cv=none; b=4DNOimdXEaR0rwbi73P9epwleMwDs80Dq60LUmkewdC+osk7rSPHl2JVYjAXJEZ4RdMkUu zKLEGfueN72BPOOox4mKkSeEXVpiiOfoAxT/+8IIQypE9F89I2lHfm2iL/2Pd5x0GZdOWu GSyWCdZtyTn8X0jBOp13zZbHlxXdZ38= Received: by mail-pl1-f175.google.com with SMTP id d9443c01a7336-1a92369761cso31818025ad.3 for ; Sun, 23 Apr 2023 15:29:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fromorbit-com.20221208.gappssmtp.com; s=20221208; t=1682288984; x=1684880984; 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=WCA3zsna6vFeV798erY3+vSi/BvHgoXGHGGpFGJHBjM=; b=CSv4RH6SXo/8LhDuQi8y+f+aMDFvLryZeZLhrGCIHJ76AP0hsBy9FXFG82OCSDXpdw w8F2b+EujbdmHkHGv/Yp2eXHQ1Q2MYfO8Vu1ERFY1LXyjQpIg0lB+5VlMI/ocJwxnhdc Xr7ocs5kYhWzgrbsToVMuFqmxnhx9PMp/1Es7VJKZagcRT2c06hsYHc4SKreGkyREEPk mCg+asSla1YXUzn1ZIV7XG9Mcyje8X207NucVVd2pMhQOzQUptnWXdlOsGXi/9ALcAP/ iq+8gIFu9tNpqRG1e620R+RCQL23t/f9n43Ntbw7lzsRcOiSQ4oQmZOQImEBwjpawJpm dy5w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682288984; x=1684880984; 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=WCA3zsna6vFeV798erY3+vSi/BvHgoXGHGGpFGJHBjM=; b=dE7TUaCFA+lbq1Md/oglCnvcdiA9dk+XjbnYlbYxcF3N1XAljwK97Bsuebxy+5v+5P aH8Gv8xuE9jAEIRYIWOXgGzEagnvT7A1BPUEWvEmbg+jOH7mHIttjhgUOXXVL8dT+G6Y 40cXqs3UeU9QVQA7IDve1u+9KPHB/qeXc3kKpp6agjPCIE+w5L4nmZoLTw6xEc30vt1Y PwSO0mT2AH5OCqdnWer/CSmbDa7w5Kr+q9xe/WUvX/3X/xzj1it0sP13wYWvADayMYWw BPDx1f+OaoaAYdoMhRl1dozoNBwR9Gh2jEZjEDFeh21amgOIQ0gt49a9N0MnabE7c7Yc XUkw== X-Gm-Message-State: AAQBX9dPqYP4mc851D4SK4zRIC0RP5M8PLrmXV3zaAO7zrgof+gKa5C8 oQiMJtRbn/yoiAdUWiuwlD4x6R9GrYxToSh9WpSSQg== X-Google-Smtp-Source: AKy350ZR4n3yiJ8OSE0TQKFE8uJrXQJX3r2CnA0Tf9NWbUzvYbwag62F1wj/UfIK9SSx06miuG3AKw== X-Received: by 2002:a17:902:e849:b0:1a6:dba5:2e3e with SMTP id t9-20020a170902e84900b001a6dba52e3emr14801275plg.25.1682288984544; Sun, 23 Apr 2023 15:29:44 -0700 (PDT) Received: from dread.disaster.area (pa49-180-41-174.pa.nsw.optusnet.com.au. [49.180.41.174]) by smtp.gmail.com with ESMTPSA id bh8-20020a170902a98800b001a641ea111fsm5444609plb.112.2023.04.23.15.29.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 23 Apr 2023 15:29:44 -0700 (PDT) Received: from dave by dread.disaster.area with local (Exim 4.92.3) (envelope-from ) id 1pqiDF-0074Ij-4U; Mon, 24 Apr 2023 08:29:41 +1000 Date: Mon, 24 Apr 2023 08:29:41 +1000 From: Dave Chinner To: Lorenzo Stoakes Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Jason Gunthorpe , Jens Axboe , Matthew Wilcox , Dennis Dalessandro , Leon Romanovsky , Christian Benvenuti , Nelson Escobar , Bernard Metzler , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Ian Rogers , Adrian Hunter , Bjorn Topel , Magnus Karlsson , Maciej Fijalkowski , Jonathan Lemon , "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Christian Brauner , Richard Cochran , Alexei Starovoitov , Daniel Borkmann , Jesper Dangaard Brouer , John Fastabend , linux-fsdevel@vger.kernel.org, linux-perf-users@vger.kernel.org, netdev@vger.kernel.org, bpf@vger.kernel.org Subject: Re: [PATCH] mm/gup: disallow GUP writing to file-backed mappings by default Message-ID: <20230423222941.GR447837@dread.disaster.area> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: C450540009 X-Rspam-User: X-Stat-Signature: 5odiqgqnq1fjetqz59xtpc7fju96zske X-HE-Tag: 1682288985-791355 X-HE-Meta: U2FsdGVkX1+kHYMge/CeBiJIj/1VrEF5sCLcTKS70IxCJUEn2DdLS6b7C1klswTUOFL9fPw3rVyJgd0xJd4xA0SW5TnEFftxy5vyIaFPgjDgHZDVSWcY/CuBp7k6U128tu0x+ufdJr37ihdxUkMB+gOSCJvi6mDx44KDYCWb23S4pW0PgJE8Eo1n7E+CjMk8YZU1xuichoYSgW+jHEqcjDaRMkYy+/VT26nOBQ7Nlw+aHbyVawP64kXKK53f28FRTA+L51mVBzYEsZWLsMHAEXV+2CjmoUg84BwC7iOXjzgVVrZLuDNSuxWDNaS5AhU4zHS4MVCuBhR//auCyTci635szY77lsPqaw8ZHkONSEIW8NwTgyrApB1hfhwZlk+LLm+H+XKOWiVmW0GcQ18UoKcl/8L3ly7Ou7Irnu7F7IAq1zmwaMPSNCpXor9Frg8Q7XUwxz/n/PhFL590XFtbMawZU9FbYgF8AIdvMbGR38ItiJ53hVWc6aGzE4eTb9NafNFHtOCNCdWdROpNZfeHDsW7VH9ou8uL0ITPwA2xrfUeRiMo/e+DGQNGgz8esTfzsUH6btzbZxViD3+jgvlt741qmx+bWBhsmt1Rcoewm+rDgJXPKR+qEiaC3YQ0PL1uxsSOPUGnZtZ+h5QwiOOwyBL1c/CcnT5+0QMIaydXL4TQnfEknBg0MY2j13aTaZopewSVU05VpR4Hyca7jzBYe9GC7dPTFne7VPplk9pOHfwoMf5VXiBXIaQ1r4VHQpLM8Ro3JHUX4hkLyI+h3c4ew9mGtvBEiCkn3WYtaeJ8xsaCDOrQ7RQNvHozM0JRHcN61mPqyv3wMLONwVY2FlyN74C7Z/R7PUoU62AWRjyphDhXiyikmeozMQ3qxrEWmUvFdmzkpKfe5q+N8aOnBhqOb3X8tp5qtPohn4xWKI5cnvb6Oi0gMd56bcyR02BffB/fDRZDqTYllUL9XEgrxat RUUOlfoO EiboEkmfS2KRdqE7IQarCrIlW5hHrkWpQvFzqPHMIhyvwrdwTtmsPir/GUTVLdPYNDCuLCyS+0wzOsFy4VVLjRCBkzswax2SoileGjcRfkqlwiCzS9HyoptEAu8DYORoIe+QRod8KXBB3kV2s02hJKsqDTQtQ8Lo2dKip7Lqkcjr7RQJj0GvejC5iyi9D4F2aB0ldWMjpROOr1myj6WGTrWdJmcz1ZK+JtFoab7XU5K1VvGAf/Uz2dguL6oRWzs/jTvknu93pUs3EcdqhqZPW0poS/pWIAdB6Y5RAfdG10cjDX/EgoWonGSvDeIsjYQceaNOVHiOBGP6PwsGF2bSumr7kZdc1X1PylvvkXqrXPDuhejLQo5zDhqblp4/m7njXmci67fmWwwA+25Oc3oGeal6N3SNweI9WAiZOgM1GcIbOhwYSQvwZhWFmqu3Mcnw+OkOCQJ9OGAy5nb+UR8yCBoi+WGlwfR5WG3jZzsWLFctllEilvslj4j9rKZBSJe4B2MaSOnRaNh99DQM= 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 Sat, Apr 22, 2023 at 02:37:05PM +0100, Lorenzo Stoakes wrote: > +/* > + * Writing to file-backed mappings using GUP is a fundamentally broken operation > + * as kernel write access to GUP mappings may not adhere to the semantics > + * expected by a file system. > + * > + * In most instances we disallow this broken behaviour, however there are some > + * exceptions to this enforced here. > + */ > +static inline bool can_write_file_mapping(struct vm_area_struct *vma, > + unsigned long gup_flags) > +{ > + struct file *file = vma->vm_file; > + > + /* If we aren't pinning then no problematic write can occur. */ > + if (!(gup_flags & (FOLL_GET | FOLL_PIN))) > + return true; > + > + /* Special mappings should pose no problem. */ > + if (!file) > + return true; Ok... > + > + /* Has the caller explicitly indicated this case is acceptable? */ > + if (gup_flags & FOLL_ALLOW_BROKEN_FILE_MAPPING) > + return true; > + > + /* shmem and hugetlb mappings do not have problematic semantics. */ > + return vma_is_shmem(vma) || is_file_hugepages(file); > +} This looks backwards. We only want the override to occur when the target won't otherwise allow it. i.e. This should be: if (vma_is_shmem(vma)) return true; if (is_file_hugepages(vma) return true; /* * Issue a warning only if we are allowing a write to a mapping * that does not support what we are attempting to do functionality. */ if (WARN_ON_ONCE(gup_flags & FOLL_ALLOW_BROKEN_FILE_MAPPING)) return true; return false; i.e. we only want the warning to fire when the override is triggered - indicating that the caller is actually using a file mapping in a broken way, not when it is being used on file/filesystem that actually supports file mappings in this way. > static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags) > { > vm_flags_t vm_flags = vma->vm_flags; > int write = (gup_flags & FOLL_WRITE); > int foreign = (gup_flags & FOLL_REMOTE); > + bool vma_anon = vma_is_anonymous(vma); > > if (vm_flags & (VM_IO | VM_PFNMAP)) > return -EFAULT; > > - if (gup_flags & FOLL_ANON && !vma_is_anonymous(vma)) > + if ((gup_flags & FOLL_ANON) && !vma_anon) > return -EFAULT; > > if ((gup_flags & FOLL_LONGTERM) && vma_is_fsdax(vma)) > @@ -978,6 +1008,10 @@ static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags) > return -EFAULT; > > if (write) { > + if (!vma_anon && > + WARN_ON_ONCE(!can_write_file_mapping(vma, gup_flags))) > + return -EFAULT; Yeah, the warning definitely belongs in the check function when the override triggers allow broken behaviour to proceed, not when we disallow a write fault because the underlying file/filesystem does not support the operation being attempted. -Dave. -- Dave Chinner david@fromorbit.com