lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 13 Dec 2019 13:17:28 -0800
From:   "Darrick J. Wong" <darrick.wong@...cle.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     y2038@...ts.linaro.org, linux-kernel@...r.kernel.org,
        linux-xfs@...r.kernel.org, Brian Foster <bfoster@...hat.com>,
        Carlos Maiolino <cmaiolino@...hat.com>,
        Pavel Reichl <preichl@...hat.com>,
        Eric Sandeen <sandeen@...deen.net>,
        Dave Chinner <dchinner@...hat.com>,
        Allison Collins <allison.henderson@...cle.com>,
        Jan Kara <jack@...e.cz>
Subject: Re: [PATCH v2 21/24] xfs: quota: move to time64_t interfaces

On Fri, Dec 13, 2019 at 09:53:49PM +0100, Arnd Bergmann wrote:
> As a preparation for removing the 32-bit time_t type and
> all associated interfaces, change xfs to use time64_t and
> ktime_get_real_seconds() for the quota housekeeping.
> 
> Signed-off-by: Arnd Bergmann <arnd@...db.de>

Looks mostly reasonable to me...

The bigtime series refactors the triplicated timer handling and whatnot,
but I don't think it would be difficult to rebase that series assuming
this lands first (which it probably will, I expect a new incompat ondisk
feature to take a /long/ time to get through review.)

> ---
>  fs/xfs/xfs_dquot.c       | 6 +++---
>  fs/xfs/xfs_qm.h          | 6 +++---
>  fs/xfs/xfs_quotaops.c    | 6 +++---
>  fs/xfs/xfs_trans_dquot.c | 8 +++++---
>  4 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 2bff21ca9d78..9cfd3209f52b 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -137,7 +137,7 @@ xfs_qm_adjust_dqtimers(
>  		    (d->d_blk_hardlimit &&
>  		     (be64_to_cpu(d->d_bcount) >
>  		      be64_to_cpu(d->d_blk_hardlimit)))) {
> -			d->d_btimer = cpu_to_be32(get_seconds() +
> +			d->d_btimer = cpu_to_be32(ktime_get_real_seconds() +
>  					mp->m_quotainfo->qi_btimelimit);
>  		} else {
>  			d->d_bwarns = 0;
> @@ -160,7 +160,7 @@ xfs_qm_adjust_dqtimers(
>  		    (d->d_ino_hardlimit &&
>  		     (be64_to_cpu(d->d_icount) >
>  		      be64_to_cpu(d->d_ino_hardlimit)))) {
> -			d->d_itimer = cpu_to_be32(get_seconds() +
> +			d->d_itimer = cpu_to_be32(ktime_get_real_seconds() +
>  					mp->m_quotainfo->qi_itimelimit);
>  		} else {
>  			d->d_iwarns = 0;
> @@ -183,7 +183,7 @@ xfs_qm_adjust_dqtimers(
>  		    (d->d_rtb_hardlimit &&
>  		     (be64_to_cpu(d->d_rtbcount) >
>  		      be64_to_cpu(d->d_rtb_hardlimit)))) {
> -			d->d_rtbtimer = cpu_to_be32(get_seconds() +
> +			d->d_rtbtimer = cpu_to_be32(ktime_get_real_seconds() +
>  					mp->m_quotainfo->qi_rtbtimelimit);

Hmm, so one thing that I clean up on the way to bigtime is the total
lack of clamping here.  If (for example) it's September 2105 and
rtbtimelimit is set to 1 year, this will cause an integer overflow.  The
quota timer will be set to 1970 and expire immediately, rather than what
I'd consider the best effort of February 2106.

(I'll grant you the current code also behaves like this...)

Reviewed-by: Darrick J. Wong <darrick.wong@...cle.com>

--D

>  		} else {
>  			d->d_rtbwarns = 0;
> diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
> index 7823af39008b..4e57edca8bce 100644
> --- a/fs/xfs/xfs_qm.h
> +++ b/fs/xfs/xfs_qm.h
> @@ -64,9 +64,9 @@ struct xfs_quotainfo {
>  	struct xfs_inode	*qi_pquotaip;	/* project quota inode */
>  	struct list_lru	 qi_lru;
>  	int		 qi_dquots;
> -	time_t		 qi_btimelimit;	 /* limit for blks timer */
> -	time_t		 qi_itimelimit;	 /* limit for inodes timer */
> -	time_t		 qi_rtbtimelimit;/* limit for rt blks timer */
> +	time64_t	 qi_btimelimit;	 /* limit for blks timer */
> +	time64_t	 qi_itimelimit;	 /* limit for inodes timer */
> +	time64_t	 qi_rtbtimelimit;/* limit for rt blks timer */
>  	xfs_qwarncnt_t	 qi_bwarnlimit;	 /* limit for blks warnings */
>  	xfs_qwarncnt_t	 qi_iwarnlimit;	 /* limit for inodes warnings */
>  	xfs_qwarncnt_t	 qi_rtbwarnlimit;/* limit for rt blks warnings */
> diff --git a/fs/xfs/xfs_quotaops.c b/fs/xfs/xfs_quotaops.c
> index c7de17deeae6..38669e827206 100644
> --- a/fs/xfs/xfs_quotaops.c
> +++ b/fs/xfs/xfs_quotaops.c
> @@ -37,9 +37,9 @@ xfs_qm_fill_state(
>  	tstate->flags |= QCI_SYSFILE;
>  	tstate->blocks = ip->i_d.di_nblocks;
>  	tstate->nextents = ip->i_d.di_nextents;
> -	tstate->spc_timelimit = q->qi_btimelimit;
> -	tstate->ino_timelimit = q->qi_itimelimit;
> -	tstate->rt_spc_timelimit = q->qi_rtbtimelimit;
> +	tstate->spc_timelimit = (u32)q->qi_btimelimit;
> +	tstate->ino_timelimit = (u32)q->qi_itimelimit;
> +	tstate->rt_spc_timelimit = (u32)q->qi_rtbtimelimit;
>  	tstate->spc_warnlimit = q->qi_bwarnlimit;
>  	tstate->ino_warnlimit = q->qi_iwarnlimit;
>  	tstate->rt_spc_warnlimit = q->qi_rtbwarnlimit;
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index a6fe2d8dc40f..d1b9869bc5fa 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -580,7 +580,7 @@ xfs_trans_dqresv(
>  {
>  	xfs_qcnt_t		hardlimit;
>  	xfs_qcnt_t		softlimit;
> -	time_t			timer;
> +	time64_t		timer;
>  	xfs_qwarncnt_t		warns;
>  	xfs_qwarncnt_t		warnlimit;
>  	xfs_qcnt_t		total_count;
> @@ -635,7 +635,8 @@ xfs_trans_dqresv(
>  				goto error_return;
>  			}
>  			if (softlimit && total_count > softlimit) {
> -				if ((timer != 0 && get_seconds() > timer) ||
> +				if ((timer != 0 &&
> +				     ktime_get_real_seconds() > timer) ||
>  				    (warns != 0 && warns >= warnlimit)) {
>  					xfs_quota_warn(mp, dqp,
>  						       QUOTA_NL_BSOFTLONGWARN);
> @@ -662,7 +663,8 @@ xfs_trans_dqresv(
>  				goto error_return;
>  			}
>  			if (softlimit && total_count > softlimit) {
> -				if  ((timer != 0 && get_seconds() > timer) ||
> +				if  ((timer != 0 &&
> +				      ktime_get_real_seconds() > timer) ||
>  				     (warns != 0 && warns >= warnlimit)) {
>  					xfs_quota_warn(mp, dqp,
>  						       QUOTA_NL_ISOFTLONGWARN);
> -- 
> 2.20.0
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ