[PATCH v3 1/1] nfs42: client needs to strip file mode's suid/sgid bit after ALLOCATE op

[PATCH v3 1/1] nfs42: client needs to strip file mode's suid/sgid bit after ALLOCATE op

[Dai Ngo]

The Linux NFS server strips the SUID and SGID from the file mode
on ALLOCATE op.

Modify _nfs42_proc_fallocate to add NFS_INO_REVAL_FORCED to
nfs_set_cache_invalid's argument to force update of the
file mode suid/sgid bit.

Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
 fs/nfs/nfs42proc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 63802d195556..9d2f07feeb29 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -81,7 +81,8 @@ static int _nfs42_proc_fallocate(struct rpc_message *msg, struct file *filep,
 	if (status == 0) {
 		if (nfs_should_remove_suid(inode)) {
 			spin_lock(&inode->i_lock);
-			nfs_set_cache_invalid(inode, NFS_INO_INVALID_MODE);
+			nfs_set_cache_invalid(inode,
+				NFS_INO_REVAL_FORCED | NFS_INO_INVALID_MODE);
 			spin_unlock(&inode->i_lock);
 		}
 		status = nfs_post_op_update_inode_force_wcc(inode,
-- 
2.9.5

Fwd: [PATCH v3 1/1] nfs42: client needs to strip file mode's suid/sgid bit after ALLOCATE op

[dai.ngo]

Hi Trond, Anna,

Is there anything else you want me to do with this patch?

Thanks,

-Dai

-------- Forwarded Message --------
Subject: 	[PATCH v3 1/1] nfs42: client needs to strip file mode's 
suid/sgid bit after ALLOCATE op
Date: 	Thu, 24 Aug 2023 16:11:47 -0700
From: 	Dai Ngo <dai.ngo@oracle.com>
To: 	trondmy@hammerspace.com, anna@kernel.org
CC: 	linux-nfs@vger.kernel.org



The Linux NFS server strips the SUID and SGID from the file mode
on ALLOCATE op.

Modify _nfs42_proc_fallocate to add NFS_INO_REVAL_FORCED to
nfs_set_cache_invalid's argument to force update of the
file mode suid/sgid bit.

Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
fs/nfs/nfs42proc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 63802d195556..9d2f07feeb29 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -81,7 +81,8 @@ static int _nfs42_proc_fallocate(struct rpc_message 
*msg, struct file *filep,
if (status == 0) {
if (nfs_should_remove_suid(inode)) {
spin_lock(&inode->i_lock);
- nfs_set_cache_invalid(inode, NFS_INO_INVALID_MODE);
+ nfs_set_cache_invalid(inode,
+ NFS_INO_REVAL_FORCED | NFS_INO_INVALID_MODE);
spin_unlock(&inode->i_lock);
}
status = nfs_post_op_update_inode_force_wcc(inode,

-- 
2.9.5

Re: [PATCH v3 1/1] nfs42: client needs to strip file mode's suid/sgid bit after ALLOCATE op

[Jeff Layton]

On Thu, 2023-08-24 at 16:11 -0700, Dai Ngo wrote:
> The Linux NFS server strips the SUID and SGID from the file mode
> on ALLOCATE op.
> 
> Modify _nfs42_proc_fallocate to add NFS_INO_REVAL_FORCED to
> nfs_set_cache_invalid's argument to force update of the
> file mode suid/sgid bit.
> 
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
>  fs/nfs/nfs42proc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index 63802d195556..9d2f07feeb29 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -81,7 +81,8 @@ static int _nfs42_proc_fallocate(struct rpc_message *msg, struct file *filep,
>  	if (status == 0) {
>  		if (nfs_should_remove_suid(inode)) {
>  			spin_lock(&inode->i_lock);
> -			nfs_set_cache_invalid(inode, NFS_INO_INVALID_MODE);
> +			nfs_set_cache_invalid(inode,
> +				NFS_INO_REVAL_FORCED | NFS_INO_INVALID_MODE);
>  			spin_unlock(&inode->i_lock);
>  		}
>  		status = nfs_post_op_update_inode_force_wcc(inode,

Yeah, I think this looks like the right thing to do. IIUC,
NFS_INO_REVAL_FORCED just means "ignore the fact that I have a
delegation", which I think is what we want here.

If this turns out to be too expensive, another idea might be to only set
FORCED here if the current mode has bits that would be cleared on a
write (i.e. setuid/setgid bits with execute bits set under them). We
don't expect "stealth" mode changes unless that's the case.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Fwd: [PATCH v3 1/1] nfs42: client needs to strip file mode's suid/sgid bit after ALLOCATE op

[dai.ngo]

Hi Anna,

Was this patch rejected? The fix was suggested by Trond.
I should have the 'Suggested-by: Trond Myklebust<trondmy@hammerspace.com>' tag.

Thanks,
-Dai



-------- Forwarded Message --------
Subject: 	[PATCH v3 1/1] nfs42: client needs to strip file mode's 
suid/sgid bit after ALLOCATE op
Date: 	Thu, 24 Aug 2023 16:11:47 -0700
From: 	Dai Ngo <dai.ngo@oracle.com>
To: 	trondmy@hammerspace.com, anna@kernel.org
CC: 	linux-nfs@vger.kernel.org



The Linux NFS server strips the SUID and SGID from the file mode
on ALLOCATE op.

Modify _nfs42_proc_fallocate to add NFS_INO_REVAL_FORCED to
nfs_set_cache_invalid's argument to force update of the
file mode suid/sgid bit.

Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
fs/nfs/nfs42proc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 63802d195556..9d2f07feeb29 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -81,7 +81,8 @@ static int _nfs42_proc_fallocate(struct rpc_message 
*msg, struct file *filep,
if (status == 0) {
if (nfs_should_remove_suid(inode)) {
spin_lock(&inode->i_lock);
- nfs_set_cache_invalid(inode, NFS_INO_INVALID_MODE);
+ nfs_set_cache_invalid(inode,
+ NFS_INO_REVAL_FORCED | NFS_INO_INVALID_MODE);
spin_unlock(&inode->i_lock);
}
status = nfs_post_op_update_inode_force_wcc(inode,

-- 
2.9.5