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 1CFEDC76195 for ; Mon, 27 Mar 2023 21:01:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A5E72900003; Mon, 27 Mar 2023 17:01:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A0DF8900002; Mon, 27 Mar 2023 17:01:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8D5C8900003; Mon, 27 Mar 2023 17:01:21 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 7F3B9900002 for ; Mon, 27 Mar 2023 17:01:21 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 383F41208CD for ; Mon, 27 Mar 2023 21:01:21 +0000 (UTC) X-FDA: 80615898762.14.4A367C6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf27.hostedemail.com (Postfix) with ESMTP id 105324001D for ; Mon, 27 Mar 2023 21:01:17 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=MUVjvo1p; spf=pass (imf27.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1679950878; a=rsa-sha256; cv=none; b=NrD2nL+1fqmPPC4fZ4fERbW/HvAb2APrcjNDlb7ElKoz5bI+JZBJh7yws55z74O9Ivvlx3 Y10nJOAStpcDoF3+DdIql+FHBECU5znMImtLmh/d7RhZ8O3eP5ibpWWKrlDsiZ4ZF7dfLL zCt+A3aZmV5rNmu4EIaJ2I3LzDG6PZg= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=MUVjvo1p; spf=pass (imf27.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1679950878; 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=pePBFHN5s0mkHf+iMKhZSRwNai0pCDXlXPyFKRXDXsc=; b=MkPimkLSJbMjQhdTtITzqQlNpSTn3Eyi3ITnrAhV+dAtDjSnDlz+RxqIYNztiOdbSV23JU dGz6taf0KlbO2pwcVgFTwCnYuqOhy/374RIxMKvRFoIjEvPoVGFks/upJ/gIRBjzLC9h4x 8ZXFIeRhVKZcIs035wSLzJ0EH/OBaGo= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1679950877; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=pePBFHN5s0mkHf+iMKhZSRwNai0pCDXlXPyFKRXDXsc=; b=MUVjvo1pdwxdHsylps3sXG7iZ/5f4LMl2PTjL7L7TvxNObywROTE6gWZcMH816tN3XeGxa R0hPo40b3vIDnXZCGrerGcRN9XdrJbIdZjRnmkqSrnQdDoGFTRa79a3CCW/heFQ7lH8pzA Sp2PAa4Dbxy28GoxiVFypA96W+9Lngo= Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-78-n9t6Cx6rOI-Yu8PQ1Mqysg-1; Mon, 27 Mar 2023 17:01:16 -0400 X-MC-Unique: n9t6Cx6rOI-Yu8PQ1Mqysg-1 Received: by mail-qt1-f197.google.com with SMTP id n10-20020a05622a11ca00b003e4e30c6c98so4079809qtk.19 for ; Mon, 27 Mar 2023 14:01:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679950875; x=1682542875; 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=pePBFHN5s0mkHf+iMKhZSRwNai0pCDXlXPyFKRXDXsc=; b=tD28YKFqevvkujUfxehFmYKIfI7qjuzOJVsXSL+l2eRfY3//zPPSWm0GoMe0T1lQ0S BGzJ83sSTu8QhQA07CVPU63bQA0kB/1v0FGZ7CjSodE2NVd2IHF5GsnPsKfIQ1prbAo1 TrHx6ZgEfgSj5ao+lMM9JgFiNnP8fSH2AEH/qayWhtGVuBOy8sMBk76ObGqg4W+ozLnU Y6eOz187Z7NAhc25skc/VEFrOUnqRuU0dgB03cM6bNt0UecMZeRkTju0984coe7QSmPe 2VyjwWSEaLGe5bNWsYOMauJ0FS0jCrpMu0Cax8HDBcWw12aSc8HjCCQw5sqzKC79TN/X aceg== X-Gm-Message-State: AAQBX9f/3KgkYJrbKgvMUBmuy0nCe9OTYhnDz+jcz8lUpiBPu7SBeZYR OL3FcdVdZOtQHqx1bquMOFweY5LVl4zk+8oTyiFadywiZ82I06pqAu91IWi8o47qCpr40KhAbvt +9Sf1HzdI/+M= X-Received: by 2002:a05:6214:528f:b0:5af:3a13:202d with SMTP id kj15-20020a056214528f00b005af3a13202dmr20867693qvb.4.1679950875514; Mon, 27 Mar 2023 14:01:15 -0700 (PDT) X-Google-Smtp-Source: AKy350aVDWYJQwWqIHwZUh5ktHNEupzgPiHGj1mY8GkaRJKKsVQEhsdqRHbLM5b4eazfi17GQ4yXtw== X-Received: by 2002:a05:6214:528f:b0:5af:3a13:202d with SMTP id kj15-20020a056214528f00b005af3a13202dmr20867635qvb.4.1679950875081; Mon, 27 Mar 2023 14:01:15 -0700 (PDT) Received: from x1n (bras-base-aurron9127w-grc-40-70-52-229-124.dsl.bell.ca. [70.52.229.124]) by smtp.gmail.com with ESMTPSA id jh19-20020a0562141fd300b005dd8b93457csm3195224qvb.20.2023.03.27.14.01.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Mar 2023 14:01:14 -0700 (PDT) Date: Mon, 27 Mar 2023 17:01:13 -0400 From: Peter Xu To: Axel Rasmussen Cc: Alexander Viro , Andrew Morton , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] userfaultfd: don't fail on unrecognized features Message-ID: References: <20220722201513.1624158-1-axelrasmussen@google.com> MIME-Version: 1.0 In-Reply-To: <20220722201513.1624158-1-axelrasmussen@google.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-Rspam-User: X-Rspamd-Queue-Id: 105324001D X-Rspamd-Server: rspam01 X-Stat-Signature: idhw9ssz1xnzcc1t8jdixspbrreo9jg4 X-HE-Tag: 1679950877-900907 X-HE-Meta: U2FsdGVkX1+pKN+M0dTXyhW9Jylzes0mMVnNtwXU3AQjEy7RNodQN//vfTxiFcdqXWpJ1DE6ORYsVvIfWlbNf/lQFaXO18KohVuz2IiklF4bijwOnTp9Wa4ebVeRNadOM+XulNkMwajPzyLu1+HZ8Hh3HvqnXGn+eirlmC1UIOokVkdlOEq6C8v1B01rUqJP0q/cTqwSteTjdnMhPxUaa2N8wOo/42zqZkWSYr+QA0FhUDYxICUu7281Oibf0ew+wwCFK8H8lDK4T8O/pJczAS+GAX784QYvPx4vE6+0/H1CH2ddtnDCdI/3kfL/HPl9AP0vX58cK3VrH2kG3UOPSXSj/hIsb7ttEVFJSIIqY43xLy86UQQGuHD/OH2tq8UtfBaGPAgKusKPaVkmHTdBBeg6dofqCrVJ4F7D58nJXMvA4IDX2+ogpPgNTtfedHTM3DpFXayViG8JKEeFP7CYOka0b96Zcxn87GE1CYfibJCqdU5+HSy6fs6AdWzqCxp3+vn8y+0TzmVFdCyg9SR5BwrUJSrJbaGe851fUjy75W12RPxde2UVlFdw7K3CssPzD2XjGyyXhWrkh0QVOdBKUE4YqIftYTZ9pcSCxNM4Kq3lib0tSHPC60v9vsWqOzC5RjcGj7vtn8LQCThfk7Gg9bTA55e4ZzmmmX9niWLU7/YyChxhPUEgrJjcRPTPSDisTbjf/fFs3Ixr74yxhUKm4unHbhYVM9geWIjHGCmS19Q0rqo0S1gqdJqkCzIF3N4YvToMfnfTRvx5r8dUkhQ1W8Pq77a2SF8P0ecJc8KJCzqxE5PNa+4bChSN+sNain5mJY8t9GT2yQ757YIQXHFVicdsjM79D0eVsQ/GNjeAliqRgpilY34P7CyCQzRsObq/m3YTyjrMvF3LTl57BTF/7/nVMRcNOfM1Ijs2nZzxzp7trDjrW52d2M3kYtLxCfekx3yzzyZ2QafNm3wC1nK KBxwka1e XIkJtx1A8e8LRVQOPfib+wHev24cdxIX/aH+4dtYDe0+ZAVRkxC2ZBCk65RoFP79mDjtz25M0+WL9L6YgG1njjF2rVJCRvaUqd2qJPTz65U00ak0bbgKVllbmtKh7Afz8h/RmLEKFe/0Szg8c6IAB+YVDW16fLQcttnRDKsWk/zDwZ/haGL0sc+zn93zT/3MkaXV35A6VQrsjLEhdVIpKSEW6XD4+/xRyjZj+G5NudHQX79xn2yQtwoqDdtsn/TfjK30foPTF3OinHExlbsRWAO8FvfeRFjCeN58Y7OSQfQYpVdyNBn0CEAVDVt5TuEnVcaKLl1Vn+KjqlJ1lwHjC3FXSjBqe6mvmniNXU4GzxVaaKM//01NjXat70QBHxKDSRlwMWKt+9IQrA5ZSuizPhl9P3A== 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: I think I overlooked this patch.. Axel, could you explain why this patch is correct? Comments inline. On Fri, Jul 22, 2022 at 01:15:13PM -0700, Axel Rasmussen wrote: > The basic interaction for setting up a userfaultfd is, userspace issues > a UFFDIO_API ioctl, and passes in a set of zero or more feature flags, > indicating the features they would prefer to use. > > Of course, different kernels may support different sets of features > (depending on kernel version, kconfig options, architecture, etc). > Userspace's expectations may also not match: perhaps it was built > against newer kernel headers, which defined some features the kernel > it's running on doesn't support. > > Currently, if userspace passes in a flag we don't recognize, the > initialization fails and we return -EINVAL. This isn't great, though. Why? IIUC that's the major way for user app to detect any misconfig of feature list so it can bail out early. Quoting from man page (ioctl_userfaultfd(2)): UFFDIO_API (Since Linux 4.3.) Enable operation of the userfaultfd and perform API handshake. ... struct uffdio_api { __u64 api; /* Requested API version (input) */ __u64 features; /* Requested features (input/output) */ __u64 ioctls; /* Available ioctl() operations (output) */ }; ... For Linux kernel versions before 4.11, the features field must be initialized to zero before the call to UFFDIO_API, and zero (i.e., no feature bits) is placed in the features field by the kernel upon return from ioctl(2). ... To enable userfaultfd features the application should set a bit corresponding to each feature it wants to enable in the features field. If the kernel supports all the requested features it will enable them. Otherwise it will zero out the returned uffdio_api structure and return EINVAL. IIUC the right way to use this API is first probe with features==0, then the kernel will return all the supported features, then the user app should enable only a subset (or all, but not a superset) of supported ones in the next UFFDIO_API with a new uffd. > Userspace doesn't have an obvious way to react to this; sure, one of the > features I asked for was unavailable, but which one? The only option it > has is to turn off things "at random" and hope something works. > > Instead, modify UFFDIO_API to just ignore any unrecognized feature > flags. The interaction is now that the initialization will succeed, and > as always we return the *subset* of feature flags that can actually be > used back to userspace. > > Now userspace has an obvious way to react: it checks if any flags it > asked for are missing. If so, it can conclude this kernel doesn't > support those, and it can either resign itself to not using them, or > fail with an error on its own, or whatever else. > > Signed-off-by: Axel Rasmussen > --- > fs/userfaultfd.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index e943370107d0..4974da1f620c 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -1923,10 +1923,8 @@ static int userfaultfd_api(struct userfaultfd_ctx *ctx, > ret = -EFAULT; > if (copy_from_user(&uffdio_api, buf, sizeof(uffdio_api))) > goto out; > - features = uffdio_api.features; > - ret = -EINVAL; > - if (uffdio_api.api != UFFD_API || (features & ~UFFD_API_FEATURES)) > - goto err_out; What's worse is that I think you removed the only UFFD_API check. Although I'm not sure whether it'll be extended in the future or not at all (very possible we keep using 0xaa forever..), but removing this means we won't be able to extend it to a new api version in the future, and misconfig of uffdio_api will wrongly succeed I think: /* Test wrong UFFD_API */ uffdio_api.api = 0xab; uffdio_api.features = 0; if (ioctl(uffd, UFFDIO_API, &uffdio_api) == 0) err("UFFDIO_API should fail but didn't"); > + /* Ignore unsupported features (userspace built against newer kernel) */ > + features = uffdio_api.features & UFFD_API_FEATURES; > ret = -EPERM; > if ((features & UFFD_FEATURE_EVENT_FORK) && !capable(CAP_SYS_PTRACE)) > goto err_out; > -- > 2.37.1.359.gd136c6c3e2-goog > -- Peter Xu