> 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
