linux-kernel - Re: [PATCH] vfs: update atimes over one day in the past or future

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]

Message-ID: <20121216021508.GG9806@dastard>
Date:	Sun, 16 Dec 2012 13:15:08 +1100
From:	Dave Chinner <david@...morbit.com>
To:	ys <sickamd@...il.com>
Cc:	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	adilger@...ger.ca
Subject: Re: [PATCH] vfs: update atimes over one day in the past or future

[ please place patches inline, not as attachments. ]

On Sat, Dec 15, 2012 at 11:25:23PM +0800, ys wrote:
> From 3d56c131b58a21c05bcd677b9d2ba915abcbf195 Mon Sep 17 00:00:00 2001
> From: yangsheng <sickamd@...il.com>
> Date: Sat, 15 Dec 2012 21:46:22 +0800
> Subject: [PATCH] vfs: update atimes over one day in the past or future
> 
> Relatime should update the inode atime if it is more than one day
> in the future.  The original problem seen was a tarball that had
> a bad atime in the distant future, but could also happen if someone
> fat-fingers a "touch".  The future atime will never be fixed.
> 
> Without relatime enabled, a future atime is updated to the current
> kernel time on access.  Relatime is meant to reduce the frequency
> of atime updates, not decide if whether the system clock or the
> inode timestamp is correct or not.
> 
> Signed-off-by: Yang Sheng <sickamd@...il.com>
> Signed-off-by: Andreas Dilger <adilger@...ger.ca>
> Acked-by: David Chinner <david@...morbit.com>

No I didn't. Please don't add tags that someone has not added
directly in a reply to the original patch.

> CC: stable@...r.kernel.org
> ---
>  fs/inode.c | 7 ++++---
>  1 ??????????????????????????? 4 ???(+)????????? 3 ???(-)

There's something wrong with the character encoding you are using...

> 
> diff --git a/fs/inode.c b/fs/inode.c
> index 14084b7..8713dc8 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1488,10 +1488,11 @@ static int relatime_need_update(struct vfsmount *mnt, struct inode *inode,
>  		return 1;
>  
>  	/*
> -	 * Is the previous atime value older than a day? If yes,
> -	 * update atime:
> +	 * Update atime if it's older than a day or more than a day
> +	 * in the future, which we assume is corrupt.
> +	 * 	 	 

A time in the future is not a corruption - the comment should
reflect exactly what you've put in the commit message. i.e. that
relatime is for reducing updates, not preventing atime from ever
moving backwards.

Also, you've added an extra line of whitespace damage that doesn't
need to be there.....

FWIW, could you write a test for xfstests for this behaviour
so we can confirm that we don't break it in future?

Cheers,

Dave.
-- 
Dave Chinner
david@...morbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/