netdev - Re: net_timedelta() affected by settimeofday() (was: [patch 12/13] net: mac80211: Remove silly timespec dance)

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: <1402564914.8095.9.camel@jlt4.sipsolutions.net>
Date:	Thu, 12 Jun 2014 11:21:54 +0200
From:	Johannes Berg <johannes@...solutions.net>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	John Stultz <john.stultz@...aro.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>,
	"John W. Linville" <linville@...driver.com>,
	linux-wireless@...r.kernel.org,
	Stephen Hemminger <shemminger@...ux-foundation.org>,
	netdev <netdev@...r.kernel.org>
Subject: Re: net_timedelta() affected by settimeofday() (was: [patch 12/13]
 net: mac80211: Remove silly timespec dance)

On Thu, 2014-06-12 at 10:57 +0200, Thomas Gleixner wrote:

> > > Right, but isn't that odd? Suddenly your delay measurement here might be
> > > minutes, hours, or years if you settimeofday() between timestamping and
> > > calculating the delta. That seems very strange to me, why would that be
> > > the right behaviour in any way?
> 
> Indeed. clock monotonic is the appropriate one for measurements.

And that's what we had here with ktime_get_ts()? I thought so, just
making sure.
 
> > > Now, it seems that there are only two current users of net_timedelta()
> > > (in DCCP) so perhaps it's not too late to change some of this?
> > > 
> > > Maybe in general the skb timestamp should be based on a different clock
> > > and only adjusted to real time when used in userspace?
> 
> You have the same problem then, just at a different place:
> 
>     ts = ktime_get();
> 
> 			settimeofday()
> 			   offset_mono_to_real = new value;
> 
>     userts = mono_to_real(ts);

Right. I'm not really sure if that's an issue though.

> But maybe that's not a real issue, as ktime_get_real() can race with
> settimeofday() or NTP as well.
> 
>    ts = ktime_get_real();
> 				settimeofday();
>    userts = ts;
> 
> So the user might see a weird timestamp for a packet, which cannot be
> correlated with the user space gettimeofday().

Right, once settimeofday() is called the timestamps from before/during
it can't really be correlated any more.

This is part of the userspace API already, but might it have been better
to expose the monotonic clock, since userspace can also get at it? Not
sure.

Either way it's an issue I guess; however I'm thinking your patch is
making it worse for the measurement in this particular code (where the
userspace issue doesn't come in, it should never be accessible there)

johannes


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html