[PATCH v2] ceph: ensure we have Fs caps when fetching dir link count

[PATCH v2] ceph: ensure we have Fs caps when fetching dir link count

[Jeff Layton]

The link count for a directory is defined as inode->i_subdirs + 2,
(for "." and ".."). i_subdirs is only populated when Fs caps are held.
Ensure we grab Fs caps when fetching the link count for a directory.

URL: https://tracker.ceph.com/issues/48125
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/inode.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 7c22bc2ea076..ab02966ef0a4 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -2343,15 +2343,23 @@ int ceph_permission(struct inode *inode, int mask)
 }
 
 /* Craft a mask of needed caps given a set of requested statx attrs. */
-static int statx_to_caps(u32 want)
+static int statx_to_caps(u32 want, umode_t mode)
 {
 	int mask = 0;
 
 	if (want & (STATX_MODE|STATX_UID|STATX_GID|STATX_CTIME|STATX_BTIME))
 		mask |= CEPH_CAP_AUTH_SHARED;
 
-	if (want & (STATX_NLINK|STATX_CTIME))
-		mask |= CEPH_CAP_LINK_SHARED;
+	if (want & (STATX_NLINK|STATX_CTIME)) {
+		/*
+		 * The link count for directories depends on inode->i_subdirs,
+		 * and that is only updated when Fs caps are held.
+		 */
+		if (S_ISDIR(mode))
+			mask |= CEPH_CAP_FILE_SHARED;
+		else
+			mask |= CEPH_CAP_LINK_SHARED;
+	}
 
 	if (want & (STATX_ATIME|STATX_MTIME|STATX_CTIME|STATX_SIZE|
 		    STATX_BLOCKS))
@@ -2377,7 +2385,7 @@ int ceph_getattr(const struct path *path, struct kstat *stat,
 
 	/* Skip the getattr altogether if we're asked not to sync */
 	if (!(flags & AT_STATX_DONT_SYNC)) {
-		err = ceph_do_getattr(inode, statx_to_caps(request_mask),
+		err = ceph_do_getattr(inode, statx_to_caps(request_mask, inode->i_mode),
 				      flags & AT_STATX_FORCE_SYNC);
 		if (err)
 			return err;
-- 
2.28.0

Re: [PATCH v2] ceph: ensure we have Fs caps when fetching dir link count

[Luis Henriques]

Jeff Layton <jlayton@kernel.org> writes:

> The link count for a directory is defined as inode->i_subdirs + 2,
> (for "." and ".."). i_subdirs is only populated when Fs caps are held.
> Ensure we grab Fs caps when fetching the link count for a directory.
>

Maybe this would be worth a stable@ tag too...?

Cheers,
-- 
Luis

> URL: https://tracker.ceph.com/issues/48125
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ceph/inode.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 7c22bc2ea076..ab02966ef0a4 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -2343,15 +2343,23 @@ int ceph_permission(struct inode *inode, int mask)
>  }
>  
>  /* Craft a mask of needed caps given a set of requested statx attrs. */
> -static int statx_to_caps(u32 want)
> +static int statx_to_caps(u32 want, umode_t mode)
>  {
>  	int mask = 0;
>  
>  	if (want & (STATX_MODE|STATX_UID|STATX_GID|STATX_CTIME|STATX_BTIME))
>  		mask |= CEPH_CAP_AUTH_SHARED;
>  
> -	if (want & (STATX_NLINK|STATX_CTIME))
> -		mask |= CEPH_CAP_LINK_SHARED;
> +	if (want & (STATX_NLINK|STATX_CTIME)) {
> +		/*
> +		 * The link count for directories depends on inode->i_subdirs,
> +		 * and that is only updated when Fs caps are held.
> +		 */
> +		if (S_ISDIR(mode))
> +			mask |= CEPH_CAP_FILE_SHARED;
> +		else
> +			mask |= CEPH_CAP_LINK_SHARED;
> +	}
>  
>  	if (want & (STATX_ATIME|STATX_MTIME|STATX_CTIME|STATX_SIZE|
>  		    STATX_BLOCKS))
> @@ -2377,7 +2385,7 @@ int ceph_getattr(const struct path *path, struct kstat *stat,
>  
>  	/* Skip the getattr altogether if we're asked not to sync */
>  	if (!(flags & AT_STATX_DONT_SYNC)) {
> -		err = ceph_do_getattr(inode, statx_to_caps(request_mask),
> +		err = ceph_do_getattr(inode, statx_to_caps(request_mask, inode->i_mode),
>  				      flags & AT_STATX_FORCE_SYNC);
>  		if (err)
>  			return err;

Re: [PATCH v2] ceph: ensure we have Fs caps when fetching dir link count

[Jeff Layton]

On Wed, 2020-11-11 at 09:34 +0000, Luis Henriques wrote:
> Jeff Layton <jlayton@kernel.org> writes:
> 
> > The link count for a directory is defined as inode->i_subdirs + 2,
> > (for "." and ".."). i_subdirs is only populated when Fs caps are held.
> > Ensure we grab Fs caps when fetching the link count for a directory.
> > 
> 
> Maybe this would be worth a stable@ tag too...?
> 
> Cheers,

Usually I reserve stable tags for "real problems" (oopses, etc), that we
want to send to mainline immediately. This is just a subtle case where
the link count in a stat() call ends up looking wrong.

If someone wants to make a case for stable, I'm willing to listen, but
this one doesn't seem worth it.

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2] ceph: ensure we have Fs caps when fetching dir link count

[Luis Henriques]

Jeff Layton <jlayton@kernel.org> writes:

> On Wed, 2020-11-11 at 09:34 +0000, Luis Henriques wrote:
>> Jeff Layton <jlayton@kernel.org> writes:
>> 
>> > The link count for a directory is defined as inode->i_subdirs + 2,
>> > (for "." and ".."). i_subdirs is only populated when Fs caps are held.
>> > Ensure we grab Fs caps when fetching the link count for a directory.
>> > 
>> 
>> Maybe this would be worth a stable@ tag too...?
>> 
>> Cheers,
>
> Usually I reserve stable tags for "real problems" (oopses, etc), that we
> want to send to mainline immediately. This is just a subtle case where
> the link count in a stat() call ends up looking wrong.
>
> If someone wants to make a case for stable, I'm willing to listen, but
> this one doesn't seem worth it.

Ok, fair enough.

Cheers,
-- 
Luis