Re: [PATCH v4 1/3] fs: don't let statmount return empty strings

[Jan Kara]

On Mon 11-11-24 10:09:55, Jeff Layton wrote:
> When one of the statmount_string() handlers doesn't emit anything to
> seq, the kernel currently sets the corresponding flag and emits an empty
> string.
> 
> Given that statmount() returns a mask of accessible fields, just leave
> the bit unset in this case, and skip any NULL termination. If nothing
> was emitted to the seq, then the EOVERFLOW and EAGAIN cases aren't
> applicable and the function can just return immediately.
> 
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  fs/namespace.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/namespace.c b/fs/namespace.c
> index ba77ce1c6788dfe461814b5826fcbb3aab68fad4..28ad153b1fb6f49653c0a85d12da457c4650a87e 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -5046,22 +5046,23 @@ static int statmount_string(struct kstatmount *s, u64 flag)
>  	size_t kbufsize;
>  	struct seq_file *seq = &s->seq;
>  	struct statmount *sm = &s->sm;
> +	u32 start = seq->count;
>  
>  	switch (flag) {
>  	case STATMOUNT_FS_TYPE:
> -		sm->fs_type = seq->count;
> +		sm->fs_type = start;
>  		ret = statmount_fs_type(s, seq);
>  		break;
>  	case STATMOUNT_MNT_ROOT:
> -		sm->mnt_root = seq->count;
> +		sm->mnt_root = start;
>  		ret = statmount_mnt_root(s, seq);
>  		break;
>  	case STATMOUNT_MNT_POINT:
> -		sm->mnt_point = seq->count;
> +		sm->mnt_point = start;
>  		ret = statmount_mnt_point(s, seq);
>  		break;
>  	case STATMOUNT_MNT_OPTS:
> -		sm->mnt_opts = seq->count;
> +		sm->mnt_opts = start;
>  		ret = statmount_mnt_opts(s, seq);
>  		break;
>  	default:
> @@ -5069,6 +5070,12 @@ static int statmount_string(struct kstatmount *s, u64 flag)
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	 * If nothing was emitted, return to avoid setting the flag
> +	 * and terminating the buffer.
> +	 */
> +	if (seq->count == start)
> +		return ret;
>  	if (unlikely(check_add_overflow(sizeof(*sm), seq->count, &kbufsize)))
>  		return -EOVERFLOW;
>  	if (kbufsize >= s->bufsize)
> 
> -- 
> 2.47.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR