Re: [PATCH] time/fs - file's time race with vgettimeofday

[Jiri Olsa]

On Tue, Jul 06, 2010 at 04:11:40PM -0700, john stultz wrote:
> On Tue, 2010-07-06 at 16:03 -0700, john stultz wrote:
> > On Fri, Jul 2, 2010 at 12:41 AM, Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> > > This issue was described in the BZ 244697
> > >
> > >        Time goes backward - gettimeofday() vs. rename()
> > >        https://bugzilla.redhat.com/show_bug.cgi?id=244697
> > >
> > >
> > > It's not just issue of the creat but few others like rename.
> > >
> > >
> > > The following patch will prevent the race by adding the
> > > CURRENT_TIME_SEC_REAL macro, which will return seconds from
> > > the getnstimeofday call, ensuring it's computed on current tick.
> > > It fixes the 'creat' case for ext4.
> > >
> > >
> > > I'm not sure how much trouble is having this race unfixed compared
> > > to the performance impact the fix might have. Maybe there're
> > > better ways to fix this.
> > 
> > I do worry that your patch will have too much of a performance hit. I
> > think the right fix would be in vtime().
> > 
> > Test patch to follow shortly.
> 
> So the following (untested) patch _should_ resolve this in mainline on
> x86. Please let me know if it works for you.
> 
> However, for older kernels, this patch won't be sufficient, as it
> depends on update_vsyscall getting the time at the last tick in its
> wall_time, and kernels that don't have the logarithmic accumulation code
> & remove xtime_cache patches can't be fixed so easily. 
> 
> Once we get this upstream, let me know if you have any questions about
> how to backport this to older kernels.
> 
> thanks
> -john
> 
> 
> 
> [PATCH] x86: Fix vtime/file timestamp inconsistencies
> 
> Due to vtime calling vgettimeofday(), its possible that an application
> could call  time();create("stuff",O_RDRW);  only to see the file's
> creation timestamp to be before the value returned by time.
> 
> The proper fix is to make vtime use the same time value as
> current_kernel_time() (which is exported via update_vsyscall) instead of
> vgettime().

hm, this would be solution for the time() call.

But I think the issue still stays for gettimeofday and clock_gettime
(CLOCK_REALTIME/CLOCK_MONOTONIC) because they call vread from
clocksource to get the real time.

Thats where I'm not sure if this race is that "bad", compared either
to performance hit or inaccuracy of user time calls.. which are possible
solutions..


jirka

> 
> 
> Signed-off-by: John Stultz <johnstul@xxxxxxxxxx>
> 
> 
> 
> diff --git a/arch/x86/kernel/vsyscall_64.c b/arch/x86/kernel/vsyscall_64.c
> index 1c0c6ab..ce9a4fa 100644
> --- a/arch/x86/kernel/vsyscall_64.c
> +++ b/arch/x86/kernel/vsyscall_64.c
> @@ -169,13 +169,18 @@ int __vsyscall(0) vgettimeofday(struct timeval * tv, struct timezone * tz)
>   * unlikely */
>  time_t __vsyscall(1) vtime(time_t *t)
>  {
> -	struct timeval tv;
> +	unsigned seq;
>  	time_t result;
>  	if (unlikely(!__vsyscall_gtod_data.sysctl_enabled))
>  		return time_syscall(t);
>  
> -	vgettimeofday(&tv, NULL);
> -	result = tv.tv_sec;
> +	do {
> +		seq = read_seqbegin(&__vsyscall_gtod_data.lock);
> +
> +		result = vsyscall_gtod_data.wall_time_sec;
> +
> +	} while (read_seqretry(&__vsyscall_gtod_data.lock, seq));
> +
>  	if (t)
>  		*t = result;
>  	return result;
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/