From nobody Mon Jun 8 14:35:15 2026 Received: from mail-ot1-f42.google.com (mail-ot1-f42.google.com [209.85.210.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A22E732B11D for ; Thu, 28 May 2026 19:22:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.42 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779996169; cv=none; b=CeQuJp5MYLY918XYw2n9r734zEBf/B7wHQu9yhuF23FEoEGKee9xkCpzC4uyNwGOly7MGnvXsjsomCUNFJx8cP6q2q8goLIvhE49ncx3W1rRzSWo1FjZJQ8cDOYLE+8Zhux530xAzPQXzYsSmm30r0oZRdPdmmeS7VpuwJpdPzk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779996169; c=relaxed/simple; bh=wFle8wdNZitY26+iINAyU/7Osr6CE2qRhx/Eo0UYvOg=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=WVK0v8SRgnalHCXesXntSxjN7a5/24x9o5hzeGcGAU/kS8JmQl+lBL22eDFiqriawfDeMVcn8VD/dpAMIy6HpmZIrJXiCxbohnVfckz0e7MHNMrm+PWkIAjAVjKKkINRyhxR/QbTZJSfkdn9vdvkjdh9jY3WmMNCOwJJkUiTvNE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=hammerspace.com; spf=fail smtp.mailfrom=hammerspace.com; dkim=pass (2048-bit key) header.d=hammerspace.com header.i=@hammerspace.com header.b=Lke6rK8p; arc=none smtp.client-ip=209.85.210.42 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=hammerspace.com Authentication-Results: smtp.subspace.kernel.org; spf=fail smtp.mailfrom=hammerspace.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=hammerspace.com header.i=@hammerspace.com header.b="Lke6rK8p" Received: by mail-ot1-f42.google.com with SMTP id 46e09a7af769-7de4a9cb8eeso11739331a34.0 for ; Thu, 28 May 2026 12:22:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hammerspace.com; s=google; t=1779996166; x=1780600966; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=y5p1jrxfDK7QO4V9U6GAswPg5bpX3/mTyGsyAQXFq0s=; b=Lke6rK8pNShgcoIkyuN02iUZw5F3Mo7kwVq2rRI+/TfPCBPz1QfozfKcMTSTppg2wO nfz/ui7XmrZzCIb3mgqqF7h9q2WBjf9njiJbJ0Q7KIQigeQSSc36XVYaXGB97s96QLoD LOMKwXLlCp+DVya2bM9gd6+2y8z4WC6otcHPkWrx4NFaEwh2JStfG507V+Cy2UC6BwFQ Gu8QTSXohqBMxSDWfS+KqD/+l23BsFa4ykLHd+PHBgCGn4kIU1nnD75Pgc+UC0JOQCtY h0J0RkqwA9/a9fmQgSy/tIeyUMKX60lME9Qxsj9yEuBk9QYuTkrgMyvxotQmQRHDNmnd Umfw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779996166; x=1780600966; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=y5p1jrxfDK7QO4V9U6GAswPg5bpX3/mTyGsyAQXFq0s=; b=LDFuMEvxSxSzRrUsZLmh6WeIrst9q+PFmiy8e4FJKL9JY1HkoKvQvQAuVWiKsqz5xK GyaZtXNdS+V9HTQfZhYhT1mXA6ucUraK1xPb002LtqYWrQxNuNb4I7/+5CJpHWc+8nOH WtfECO5twFLNC2/tq+zj4AXFDqjwlYc2/BqYdzr2+rXQizIUEs6Ctmzh/VR/YAgD+Aok ppfsKoczUVM0N5fT8FiSLasc+YycKmuzeGun7c23hadv0begzs5Er7as0w02QO3SmpMA WVwSVECYcxubOg9PMrFqutVOMPkeamT2WbRKrAFbN+xjiIaTrB5Izeq2qYM/gxLe6tz6 VU5g== X-Forwarded-Encrypted: i=1; AFNElJ+CKs8kniXS4GZsR1Ib3BZLy2X+bOWyDnLEK19hcqjLXZRKP8WGNXTnU6AmmNaZIovk9gKiDj0UaDFs8qE=@vger.kernel.org X-Gm-Message-State: AOJu0Yx+c1RXbkema4xZt+gO1oyuiUq82a0R6v/BXQ7kMkFG7JgCph9h CnxbceJEvdxAssm+zwGGRTF3em0tm1hkOjKdu4ysF9PEVlE/J4BaCnRzsqXo56m6VNU= X-Gm-Gg: Acq92OGiY+2VibfP+UXspS30R7Xrw2gD2w62ECjUTLd0HqZ4XpQd/dyjfZkDKH+gU6o V/Z9wFVMW0qGC7MxT9mePASwqWWzAQ7ulY7/d7S4IXxnsKSOrhuo8c8oPb6HUfHuycxglJhRxCt RxOzLVqbNw15GoMJ8YJU4WV8fftjRIQwf7odm0j9cjsfrt2MLKiCpp+KORHINcLK3ErVN+MMB/z hwuprKYHXUvhgAEYNTx1/E4cT+fh9LUglBL40MI5FjcC1Ey7s0vyyHXqaAWkOuVUJ/LSXXaHLiR ePvTYHOzdygZ70fbSQe8p44/gwtCeanSA3u/wCRx7eVMMq3UduyZsTKRAmfaao11ivYpL4YVCf3 Non6Ubpl6jvXkagTBY0Nok1Dfd3HZf0gG5CWX+yrUy/3bP8mUgaFoXZF2Rscdvx0+TIMQSV3Njv 5NmWl2G5Jju3WsN5ZBihw4YfnTl+Ie7lvn8gtNtEy0VHzFOvjW043hOudDGeVEv0NN3UrGyCI5d m4Iuwel X-Received: by 2002:a05:6830:4104:b0:7dc:c4ae:a679 with SMTP id 46e09a7af769-7e6944f6978mr21854a34.9.1779996165551; Thu, 28 May 2026 12:22:45 -0700 (PDT) Received: from bcodding.csb.hammerspace.com ([66.97.168.37]) by smtp.gmail.com with ESMTPSA id 46e09a7af769-7e6064aa0bbsm15006127a34.12.2026.05.28.12.22.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 May 2026 12:22:44 -0700 (PDT) From: Benjamin Coddington X-Google-Original-From: Benjamin Coddington To: Trond Myklebust , Anna Schumaker Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 1/1] nfs: return a write delegation when a SETATTR drops our write access Date: Thu, 28 May 2026 15:22:42 -0400 Message-ID: <64a9c99c387432399b4c4d9ce6dd4836b0170c15.1779995818.git.bcodding@hammerspace.com> X-Mailer: git-send-email 2.53.0 In-Reply-To: References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" A client holding an OPEN_DELEGATE_WRITE delegation can satisfy a later open(O_WRONLY) from the cached delegation (can_open_delegated()) without sending an OPEN to the server. That cached "open for write" assertion is only valid while the delegation holder still has write access. A SETATTR that changes mode, owner, or group can revoke that access -- after which an open served from the delegation would bypass an access check the server would now fail, and, against a server that recalls the delegation on such a change, the SETATTR draws a CB_RECALL/NFS4ERR_DELAY/DELEGRETURN/retry round trip. Before issuing such a SETATTR, check whether the proposed mode/owner/group would remove write access for the delegation's owning credential, judged by the resulting POSIX mode bits. If so, return the delegation first: the return is synchronous and flushes modified data, so the SETATTR proceeds on an open or special stateid and the next open revalidates access with the server. Permission changes that keep the holder's write access leave the delegation in place. Only the mode bits and the holder's fsuid/fsgid are consulted. An NFSv4 ACL cannot be evaluated by the client, a privileged caller may retain access the bits deny, and supplementary group membership is not checked, so the test is necessarily approximate -- but an inexact answer costs at most an unnecessary delegation return or a fall back to the server's recall, never incorrect access. RFC 8881 Section 10.4.4 permits a client to return a delegation voluntarily, performing the same pre-return state updates (data flush, pending truncation, CLOSE/OPEN/LOCK) it would on a recall. Commit c01d36457dcc ("NFSv4: Don't return the delegation when not needed by NFSv4.x (x>0)") stopped returning write delegations on SETATTR for NFSv4.1+, since the server can identify the delegation holder from the SEQUENCE clientid and need not recall. That holds for changes that do not affect the holder's access; restore a return only for the narrow case where the holder's own write access is removed. Signed-off-by: Benjamin Coddington --- fs/nfs/nfs4proc.c | 66 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 62 insertions(+), 4 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index a9b8d482d289..e4b7322bf75c 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -4506,7 +4506,55 @@ int nfs4_proc_getattr(struct nfs_server *server, str= uct nfs_fh *fhandle, return err; } =20 -/*=20 +/* + * Would applying @sattr (which changes mode, owner, and/or group) remove = the + * write access of a held write delegation's owning credential, as judged = by + * the resulting file mode bits? + * + * Such a change makes the delegation's cached "open for write" assertion + * stale: a later open(O_WRONLY) could be served from the delegation witho= ut + * the server getting a chance to deny it. Only the mode bits and the + * holder's fsuid/fsgid are consulted; an NFSv4 ACL (which the client cann= ot + * evaluate locally), a privileged caller, or supplementary group membersh= ip + * may make the answer imprecise, but the cost is at most an unnecessary + * delegation return or a fall back to the server's recall -- never incorr= ect + * access. + */ +static bool nfs4_setattr_removes_write(struct inode *inode, struct iattr *= sattr) +{ + struct nfs_delegation *delegation; + const struct cred *cred; + umode_t mode =3D inode->i_mode; + kuid_t uid =3D inode->i_uid; + kgid_t gid =3D inode->i_gid; + bool ret =3D false; + + delegation =3D nfs4_get_valid_delegation(inode); + if (!delegation) + return false; + if (!(delegation->type & FMODE_WRITE)) + goto out; + cred =3D delegation->cred; + + if (sattr->ia_valid & ATTR_MODE) + mode =3D sattr->ia_mode; + if (sattr->ia_valid & ATTR_UID) + uid =3D sattr->ia_uid; + if (sattr->ia_valid & ATTR_GID) + gid =3D sattr->ia_gid; + + if (uid_eq(uid, cred->fsuid)) + ret =3D !(mode & S_IWUSR); + else if (gid_eq(gid, cred->fsgid)) + ret =3D !(mode & S_IWGRP); + else + ret =3D !(mode & S_IWOTH); +out: + nfs_put_delegation(delegation); + return ret; +} + +/* * The file is not closed if it is opened due to the a request to change * the size of the file. The open call will not be needed once the * VFS layer lookup-intents are implemented. @@ -4555,9 +4603,19 @@ nfs4_proc_setattr(struct dentry *dentry, struct nfs_= fattr *fattr, cred =3D ctx->cred; } =20 - /* Return any delegations if we're going to change ACLs */ - if ((sattr->ia_valid & (ATTR_MODE|ATTR_UID|ATTR_GID)) !=3D 0) - nfs4_inode_make_writeable(inode); + /* + * A change to mode, owner, or group that removes the write + * delegation holder's own write access makes the delegation's cached + * "open for write" stale; return it so a later open() revalidates + * access with the server. A change that keeps write access leaves + * the delegation in place. + */ + if (sattr->ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)) { + if (nfs4_setattr_removes_write(inode, sattr)) + nfs4_inode_return_delegation(inode); + else + nfs4_inode_make_writeable(inode); + } =20 status =3D nfs4_do_setattr(inode, cred, fattr, sattr, ctx, NULL); if (status =3D=3D 0) { --=20 2.53.0