netdev - RE: [PATCH net V4 2/2] igb: offer a PTP Hardware Clock instead of the timecompare method

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, 23 Mar 2012 18:27:06 +0000
From:	"Keller, Jacob E" <jacob.e.keller@...el.com>
To:	chetan loke <loke.chetan@...il.com>,
	Richard Cochran <richardcochran@...il.com>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"e1000-devel@...ts.sourceforge.net" 
	<e1000-devel@...ts.sourceforge.net>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"Ronciak, John" <john.ronciak@...el.com>,
	"john.stultz@...aro.org" <john.stultz@...aro.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>
Subject: RE: [PATCH net V4 2/2] igb: offer a PTP Hardware Clock instead of
 the timecompare method

> -----Original Message-----
> From: chetan loke [mailto:loke.chetan@...il.com]
> Sent: Thursday, March 22, 2012 4:14 PM
> To: Richard Cochran
> Cc: Keller, Jacob E; netdev@...r.kernel.org; e1000-
> devel@...ts.sourceforge.net; Kirsher, Jeffrey T; Ronciak, John;
> john.stultz@...aro.org; tglx@...utronix.de
> Subject: Re: [PATCH net V4 2/2] igb: offer a PTP Hardware Clock instead of the
> timecompare method
> 
> // trip cnt will ensure/enforce - evil adjtime user-space code can still not
> block us.
> // called from igb_tx[rx]_hwtstamp
> driver_rx_tx_path_locking( ... ) {
>      unsigned int seq, trip_cnt = 0;
>      u64 ns;
>      do {
>      seq = read_seqbegin( &pigb->tmreg_seq_lock);
>      trip_cnt++;
>      ns = timecounter_read(&pigb->tc);
These functions would use timecounter_cyc2time, instead, and convert the RXTSTAMPL/H registers. That prevents the issue that Richard mentions (here), because two packets can't be timestamped at the same time anyways. (and on the devices where they can, the timestamp isn't returned via registers

However the try_cnt would not work, because of a case where the timecounter is just about to roll over. (So last check was 25 seconds ago.) Then the cyc2time function is called, determines that some RX timestamp is in the 'future'. At this point on another CPU the timecounter_read is run, and updates the nsec value in the timecounter by 25 seconds, and stores the last read timestamp value. Now the cyc2time function uses the 'new' values, but doesn't know to correct for it by using the "in the past" subtraction. Thus, the resulting ns returned by cyc2time would be 25 seconds in the future. This isn't just not accurate, it is very wrong and could throw of PTP calculations.

The solution here is when the try_cnt runs out, drop the timestamp or return 0 as the value. (So that the PTP app knows the timestamp was bad.) However this is introducing more potentially dropped timestamps,  which could be an issue.

I think a better way to solve this problem is throttle the ioctls. Would it be possible to detect users spamming the ioctls and return some sort of 'EBUSY' error? Maybe by using spin_trylock? That would prevent the starvation issues mentioned if a user were to spam set/get/adj in some weird loop, without a potentially dangerous locking scheme.