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 2BA5CC5AE59 for ; Wed, 4 Jun 2025 00:52:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6BB226B0537; Tue, 3 Jun 2025 20:52:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 693616B0538; Tue, 3 Jun 2025 20:52:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5AB456B0539; Tue, 3 Jun 2025 20:52:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 3C82E6B0537 for ; Tue, 3 Jun 2025 20:52:52 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id A7776161B3F for ; Wed, 4 Jun 2025 00:52:51 +0000 (UTC) X-FDA: 83515893342.12.AAD4D5A Received: from mail-yb1-f181.google.com (mail-yb1-f181.google.com [209.85.219.181]) by imf11.hostedemail.com (Postfix) with ESMTP id C8FD240005 for ; Wed, 4 Jun 2025 00:52:49 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=NYjxgLv3; spf=pass (imf11.hostedemail.com: domain of jthoughton@google.com designates 209.85.219.181 as permitted sender) smtp.mailfrom=jthoughton@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1748998369; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=ufdJ7zCVJhXmyO6Hnb5W+3LfKFzQXrWMg7EkU2xzrdU=; b=pnkkidkLrI9Gizcada6QVY2klOdOTp/tdKT/cPkxZpoLJ1BaXQZYWbTLHAw8dlEPeVvhSA DbG7iY9+KDRKwEtsSC4xDVBPEqoYxyjcBF5Rue9fmj5AUe63MwddLzLgyUF1vZcwOtiB2C D/cF4qk1FUvZVKfxkspWMDanvTcm9Mo= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=NYjxgLv3; spf=pass (imf11.hostedemail.com: domain of jthoughton@google.com designates 209.85.219.181 as permitted sender) smtp.mailfrom=jthoughton@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1748998369; a=rsa-sha256; cv=none; b=VdUmOzDRtIOZltWGt/wvH6s+v0u3FTpqszU65YvxbFIpqHa1mp3Uw+HOqOAi0p0uDCPUHT WqV0k4VkWwO/UC5VAa0uINQzOjang2StUH7BEL+ON3TBmx6JB8sT5Mwwr2G7tpdWSNyESR SJ2Ku1Hon785VOIsTowVzvAMICjPg8U= Received: by mail-yb1-f181.google.com with SMTP id 3f1490d57ef6-e8134dee405so3305905276.1 for ; Tue, 03 Jun 2025 17:52:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1748998369; x=1749603169; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=ufdJ7zCVJhXmyO6Hnb5W+3LfKFzQXrWMg7EkU2xzrdU=; b=NYjxgLv3zSI30wLmJIxY6ab4zeeYwT0iu4Dh0YhdIwQUHVS1hQ8Id5fh4f8FqJuHyL 8EiXjxhmhdg5Rh2qFF7ti8rV+xb9IbTdZRqK2MceftPbr5f3pwLLRfp7L06pI1btxFQ6 wxAVsWjRcX5H7MEyhiTXORBoCZ+r73cPhpnjEEUKD+3nCdxKwlc7O/jAd4L67ciJnfzX MWbyrqhOEwdzLkY6ZuoHx11tQOzLEMQouCh+LK4ozMMHYeSvoN4igssCc51OwbP7S8qm PiRzdiyaUTBxnjjgIoN4bJY/9yqyLqukdpO55yyW8saWA5oPqchMFYNaMo16FG1e2gEM QLTQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1748998369; x=1749603169; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ufdJ7zCVJhXmyO6Hnb5W+3LfKFzQXrWMg7EkU2xzrdU=; b=H9ADNIRB5B2RkXzWklppA4C+W24kTSj4PQAZTnJggtu/gc5910DtiEpfbvSoMakbkd HP7i+8yKh914qNEy3DfCe7LXczb5kJnVurIHKOK8AB+1WpacQm97QFSIfL5VDnvN/Ne/ xqq4TCp8SygLUY/y6669Z8nkPVkvBrOy/vkyXxSRhgnOwIB8HidYIPl8di52yS10op8+ IEQd+s2rJATHDt/wAMHA0xm+uaqBwEi5gNiK+iAJLrynXexw4qTvEY6GhsvNs4JWDwwI m+wcWts5GZgs5kfR6A2k4S+bR6wVsQGhlF6gGcnoScE/rJycwXQbJAFcrRUhI6K1K5g1 U04g== X-Forwarded-Encrypted: i=1; AJvYcCWEhG34i6+vUu9gQ4GsuWH3SFNQj3iOXc/zXUJoc2kBawkkAaO3ldj9bFWA+zrQxbZZ1F8gfIqnsA==@kvack.org X-Gm-Message-State: AOJu0YxZbPoD8ULP90cH187P5XcbDtVZdwCtfuocKGSjm4PKwswolYAF RBlNF/aAA07dFoRKUV6EWbKlNwyZFhWlOyPFPqM48esruRdotZun1+MIPn0HyCFy1CuWSWJczyd PcjWXjoXuHFv+pZjfPi6Iz4mq8MzzW/ZPuRHUXU9a X-Gm-Gg: ASbGncvJ31YSeQoMFjvFG5RWoyUauNOSk5lpExeZlnGUtnsm7hX6DpS5pVw//zEadea hzEF7u6TNfwltJdXczZloc887LwfrZkH2YI8zupTH+7wVd70TXwatA6dGTS/3d02sFBKtiuAQTB w8jceq6TyvB3WakTETJeWeLmx3ok8ekERSAGvXmjWJhKEEO7sIvXi6HX6FMA9L1a3ey67Apk8MZ A== X-Google-Smtp-Source: AGHT+IEnDQtfQo5x5DIJYs2UH2zoxENxePjxDZ00t7iisvDN9qp9tZqmkNYduYRzTjcj7MnmSchmbiJHQmlMkZYa+Go= X-Received: by 2002:a05:6902:18d0:b0:e7d:c87a:6249 with SMTP id 3f1490d57ef6-e8179dbababmr1390646276.36.1748998368683; Tue, 03 Jun 2025 17:52:48 -0700 (PDT) MIME-Version: 1.0 References: <20250603-uffd-fixes-v1-0-9c638c73f047@columbia.edu> <20250603-uffd-fixes-v1-2-9c638c73f047@columbia.edu> In-Reply-To: <20250603-uffd-fixes-v1-2-9c638c73f047@columbia.edu> From: James Houghton Date: Tue, 3 Jun 2025 17:52:12 -0700 X-Gm-Features: AX0GCFvhX97qdJoaP_U5MjbCXqe-defL5vJZdiAvYUJ8X717AX3GBN57TvbH-7A Message-ID: Subject: Re: [PATCH 2/3] userfaultfd: prevent unregistering VMAs through a different userfaultfd To: Tal Zussman Cc: Andrew Morton , Peter Xu , "Jason A. Donenfeld" , David Hildenbrand , Alexander Viro , Christian Brauner , Jan Kara , Pavel Emelyanov , Andrea Arcangeli , linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: C8FD240005 X-Stat-Signature: ycisit1o7c6zwsxbpubg9w1fne4b4uxh X-Rspam-User: X-HE-Tag: 1748998369-603413 X-HE-Meta: U2FsdGVkX18n8NYird9aBs3+0fCLPrC+EMZKqp6Px5yqD0CTqd/nzEbdFoQX/Me6nKuraIPZrt/fqLtkowxq1Y42mG+9ytQBTMg4i2F3vauX/AsysLPJwwz6BXf8yHY2jGeWxdypppBoirfP4W2lT2k0X/01DYmAv7kWGd35nITObPVyqOV0ojoaBXwRoCUKAcL5x1fo9X6tLxuQQMBSQfqjwclksbG5ywc5z+nugIh06syc8rC8GoFvpxnxCSH6Eh4ElEsIHn7zT59NQo7O2Qc1vluGAwPplQXnY+Cp6adw4bmnFy2X5HBdoNtrBRD05/+X4X5T+a7NdI3W2NpbzISUevieYZIQffnb7idNLnNKNtml+oipT8+Lm9QTrDjSFbK8+x3pUYFOMSm3W4K6g12oOSzJsfog2hj/7XZTXDKuorhNtTQzXJy3xxMwjJVZCHIavWMDG564SA6JwxqXOwg9uV5ayc9qRIdO0iXNqXa+HrcOeUbNT+kArgVKcVy9LRETfBwXBEDyFfoA/MQIY/Q6aHjhZ875ArVhE5g/dW7TgsM1sGpNEkU8DA/rJM+dykNMfKRRu9wypN6ayJ+CaTmaI8QgEti6EJrMcib/N1wSP3QRgweR+88WUSvijqAaJwuqtznRwzkpVB77n5X9sbstOa7/eYApx7p6SNwksiTQZdN4ifIbcWc4EwBM3N+o9HsK/RhHTqG6zwnH6UTi2h1TEEqprXzmO8Kt2pf0TGMAbuFwiG4BJS6hIR0lYvFheBCR5tHSKrzqBZyL9NmxA9kCvYAKbTaffYd0HfnHb+jrmmS7Esg8lInq8pPZKsovo0GlPPcIOrJRWBL7T4hsaCp9LitdmzvyTLN0rdSYXVhV59pbSrnm/jRPSLmhTVeM1pFfrDrcelcuJORN1H98Y6VuHmXZG4CXApEfat1bB7pcmv/8/xycFVqBBOsapLk05dgStwKEw18s1qvBPFE mZJfGdHc zd5voekd1JztsCHG736+iPD8pY9UKLTAZoLMFt5Ym1UI4Wx0Srwvmqpmt9OHrQuTR4g/x7cDsEaE7ZmhzG0olKkJNhfE/e9XEP39yNZGyTyYnf7aKpRG2t7Pk/yAgWQvHguwmFMgloQC0gqibUO1B4J9IMR9DLFTBgLNd01sbWg73l5WXJZsk97vQ7azQtEnOg3mi 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: List-Subscribe: List-Unsubscribe: On Tue, Jun 3, 2025 at 3:15=E2=80=AFPM Tal Zussman wr= ote: > > Currently, a VMA registered with a uffd can be unregistered through a > different uffd asssociated with the same mm_struct. > > Change this behavior to be stricter by requiring VMAs to be unregistered > through the same uffd they were registered with. > > While at it, correct the comment for the no userfaultfd case. This seems > to be a copy-paste artifact from the analagous userfaultfd_register() > check. > > Fixes: 86039bd3b4e6 ("userfaultfd: add new syscall to provide memory exte= rnalization") > Signed-off-by: Tal Zussman Thanks, Tal! I like this patch, but I can't really meaningfully comment on if it's worth it to change the UAPI. > --- > fs/userfaultfd.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index 22f4bf956ba1..9289e30b24c4 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -1477,6 +1477,16 @@ static int userfaultfd_unregister(struct userfault= fd_ctx *ctx, > if (!vma_can_userfault(cur, cur->vm_flags, wp_async)) > goto out_unlock; > > + /* > + * Check that this vma isn't already owned by a different > + * userfaultfd. This provides for more strict behavior by > + * preventing a VMA registered with a userfaultfd from be= ing > + * unregistered through a different userfaultfd. > + */ > + if (cur->vm_userfaultfd_ctx.ctx && > + cur->vm_userfaultfd_ctx.ctx !=3D ctx) > + goto out_unlock; > + Very minor nitpick: I think this check should go above the !vma_can_userfault() check above, as `wp_async` was derived from `ctx`, not `cur->vm_userfaultfd_ctx.ctx`. > found =3D true; > } for_each_vma_range(vmi, cur, end); I don't really like this for_each_vma_range() for loop, but I guess it is meaningful to the user: invalid unregistration attempts will fail quickly instead of potentially making some progress. So unfortunately, without a good reason, I suppose we can't get rid of it. :( > BUG_ON(!found); > @@ -1491,10 +1501,11 @@ static int userfaultfd_unregister(struct userfaul= tfd_ctx *ctx, > cond_resched(); > > BUG_ON(!vma_can_userfault(vma, vma->vm_flags, wp_async)); > + BUG_ON(vma->vm_userfaultfd_ctx.ctx && > + vma->vm_userfaultfd_ctx.ctx !=3D ctx); IMO, this new BUG_ON should either be (1) moved and should not be a BUG_ON. See the WARN_ON_ONCE() below, OR (2) removed. Perhaps the older BUG_ON() should be removed/changed too. > > /* > - * Nothing to do: this vma is already registered into thi= s > - * userfaultfd and with the right tracking mode too. > + * Nothing to do: this vma is not registered with userfau= ltfd. > */ > if (!vma->vm_userfaultfd_ctx.ctx) > goto skip; if (WARN_ON_ONCE(vmx->vm_userfaultfd_ctx.ctx !=3D ctx)) { ret =3D -EINVAL; break; } where the WARN_ON_ONCE() indicates that the VMA should have been filtered out earlier. The WARN_ON_ONCE() isn't even really necessary. > > -- > 2.39.5 > >