Re: [PATCH] SUNRPC: return proper error from get_expiry()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On Mar 8, 2023, at 1:51 AM, NeilBrown <neilb@xxxxxxx> wrote:
> 
> 
> The get_expiry() function currently returns a timestamp, and uses the
> special return value of 0 to indicate an error.
> 
> Unfortunately this causes a problem when 0 is the correct return value.
> 
> On a system with no RTC it is possible that the boot time will be seen
> to be "3".  When exportfs probes to see if a particular filesystem
> supports NFS export it tries to cache information with an expiry time of
> "3".  The intention is for this to be "long in the past".  Even with no
> RTC it will not be far in the future (at most a second or two) so this
> is harmless.
> But if the boot time happens to have been calculated to be "3", then
> get_expiry will fail incorrectly as it converts the number to "seconds
> since bootime" - 0.
> 
> To avoid this problem we change get_expiry() to report the error quite
> separately from the expiry time.  The error is now the return value.
> The expiry time is reported through a by-reference parameter.
> 
> Reported-by: Jerry Zhang <jerry@xxxxxxxxxx>
> Tested-by: Jerry Zhang <jerry@xxxxxxxxxx>
> Signed-off-by: NeilBrown <neilb@xxxxxxx>

Thanks, Neil. Applied to nfsd-next.


> ---
> fs/nfsd/export.c                  | 13 ++++++-------
> fs/nfsd/nfs4idmap.c               |  8 ++++----
> include/linux/sunrpc/cache.h      | 15 ++++++++-------
> net/sunrpc/auth_gss/svcauth_gss.c | 12 ++++++------
> net/sunrpc/svcauth_unix.c         | 12 ++++++------
> 5 files changed, 30 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index 668c7527b17e..6da74aebe1fb 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -123,11 +123,11 @@ static int expkey_parse(struct cache_detail *cd, char *mesg, int mlen)
> 
> 	/* OK, we seem to have a valid key */
> 	key.h.flags = 0;
> -	key.h.expiry_time = get_expiry(&mesg);
> -	if (key.h.expiry_time == 0)
> +	err = get_expiry(&mesg, &key.h.expiry_time);
> +	if (err)
> 		goto out;
> 
> -	key.ek_client = dom;	
> +	key.ek_client = dom;
> 	key.ek_fsidtype = fsidtype;
> 	memcpy(key.ek_fsid, buf, len);
> 
> @@ -610,9 +610,8 @@ static int svc_export_parse(struct cache_detail *cd, char *mesg, int mlen)
> 	exp.ex_devid_map = NULL;
> 
> 	/* expiry */
> -	err = -EINVAL;
> -	exp.h.expiry_time = get_expiry(&mesg);
> -	if (exp.h.expiry_time == 0)
> +	err = get_expiry(&mesg, &exp.h.expiry_time);
> +	if (err)
> 		goto out3;
> 
> 	/* flags */
> @@ -624,7 +623,7 @@ static int svc_export_parse(struct cache_detail *cd, char *mesg, int mlen)
> 		if (err || an_int < 0)
> 			goto out3;
> 		exp.ex_flags= an_int;
> -	
> +
> 		/* anon uid */
> 		err = get_int(&mesg, &an_int);
> 		if (err)
> diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
> index 5e9809aff37e..7a806ac13e31 100644
> --- a/fs/nfsd/nfs4idmap.c
> +++ b/fs/nfsd/nfs4idmap.c
> @@ -240,8 +240,8 @@ idtoname_parse(struct cache_detail *cd, char *buf, int buflen)
> 		goto out;
> 
> 	/* expiry */
> -	ent.h.expiry_time = get_expiry(&buf);
> -	if (ent.h.expiry_time == 0)
> +	error = get_expiry(&buf, &ent.h.expiry_time);
> +	if (error)
> 		goto out;
> 
> 	error = -ENOMEM;
> @@ -408,8 +408,8 @@ nametoid_parse(struct cache_detail *cd, char *buf, int buflen)
> 	memcpy(ent.name, buf1, sizeof(ent.name));
> 
> 	/* expiry */
> -	ent.h.expiry_time = get_expiry(&buf);
> -	if (ent.h.expiry_time == 0)
> +	error = get_expiry(&buf, &ent.h.expiry_time);
> +	if (error)
> 		goto out;
> 
> 	/* ID */
> diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
> index ec5a555df96f..518bd28f5ab8 100644
> --- a/include/linux/sunrpc/cache.h
> +++ b/include/linux/sunrpc/cache.h
> @@ -300,17 +300,18 @@ static inline int get_time(char **bpp, time64_t *time)
> 	return 0;
> }
> 
> -static inline time64_t get_expiry(char **bpp)
> +static inline int get_expiry(char **bpp, time64_t *rvp)
> {
> -	time64_t rv;
> +	int error;
> 	struct timespec64 boot;
> 
> -	if (get_time(bpp, &rv))
> -		return 0;
> -	if (rv < 0)
> -		return 0;
> +	error = get_time(bpp, rvp);
> +	if (error)
> +		return error;
> +
> 	getboottime64(&boot);
> -	return rv - boot.tv_sec;
> +	(*rvp) -= boot.tv_sec;
> +	return 0;
> }
> 
> #endif /*  _LINUX_SUNRPC_CACHE_H_ */
> diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
> index acb822b23af1..bfaf584d296a 100644
> --- a/net/sunrpc/auth_gss/svcauth_gss.c
> +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> @@ -258,11 +258,11 @@ static int rsi_parse(struct cache_detail *cd,
> 
> 	rsii.h.flags = 0;
> 	/* expiry */
> -	expiry = get_expiry(&mesg);
> -	status = -EINVAL;
> -	if (expiry == 0)
> +	status = get_expiry(&mesg, &expiry);
> +	if (status)
> 		goto out;
> 
> +	status = -EINVAL;
> 	/* major/minor */
> 	len = qword_get(&mesg, buf, mlen);
> 	if (len <= 0)
> @@ -484,11 +484,11 @@ static int rsc_parse(struct cache_detail *cd,
> 
> 	rsci.h.flags = 0;
> 	/* expiry */
> -	expiry = get_expiry(&mesg);
> -	status = -EINVAL;
> -	if (expiry == 0)
> +	status = get_expiry(&mesg, &expiry);
> +	if (status)
> 		goto out;
> 
> +	status = -EINVAL;
> 	rscp = rsc_lookup(cd, &rsci);
> 	if (!rscp)
> 		goto out;
> diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
> index b1efc34db6ed..9e7798a69051 100644
> --- a/net/sunrpc/svcauth_unix.c
> +++ b/net/sunrpc/svcauth_unix.c
> @@ -225,9 +225,9 @@ static int ip_map_parse(struct cache_detail *cd,
> 		return -EINVAL;
> 	}
> 
> -	expiry = get_expiry(&mesg);
> -	if (expiry ==0)
> -		return -EINVAL;
> +	err = get_expiry(&mesg, &expiry);
> +	if (err)
> +		return err;
> 
> 	/* domainname, or empty for NEGATIVE */
> 	len = qword_get(&mesg, buf, mlen);
> @@ -497,9 +497,9 @@ static int unix_gid_parse(struct cache_detail *cd,
> 	uid = make_kuid(current_user_ns(), id);
> 	ug.uid = uid;
> 
> -	expiry = get_expiry(&mesg);
> -	if (expiry == 0)
> -		return -EINVAL;
> +	err = get_expiry(&mesg, &expiry);
> +	if (err)
> +		return err;
> 
> 	rv = get_int(&mesg, &gids);
> 	if (rv || gids < 0 || gids > 8192)
> -- 
> 2.39.2
> 

--
Chuck Lever






[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux