Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#4153 closed Bug (fixed)

Protect against non-monotonic time in libutp

Reported by: jch Owned by: jch
Priority: High Milestone:
Component: libtransmission Version: 2.22+
Severity: Normal Keywords: libutp upstream
Cc:

Description

Libutp currently assumes that utp_GetMicroseconds returns monotonic time, i.e. that it never goes backwards. If it ever does, it will crash with an assertion violation.

In https://github.com/bittorrent/libutp/commit/a081cfbc8726e6032717b621ab6a7a7514ca5b69 , I've made libutp use real time instead of monotonic time if there are no POSIX clocks in the kernel, e.g. on Linux 2.4. This implies that on such a system, we'll crash if somebody steps the time backwards.

The fix provided in #4090 is *not* right, since libutp makes this assumption in many other places. The right fix is to wrap getMicroseconds in some stuff that makes sure it returns monotonic values. This should be done upstream.

Jordan, please treat this as a blocker for 2.30 unless you release with µTP disabled.

--jch

Change History (15)

comment:1 Changed 8 years ago by jch

Hopefully fixed in r12259.

--jch

comment:2 Changed 8 years ago by jch

  • Resolution set to fixed
  • Status changed from new to closed

comment:3 Changed 8 years ago by jch

Amended by r12260.

--jch

comment:4 Changed 8 years ago by livings124

  • Version changed from 2.22 to 2.22+

comment:5 Changed 8 years ago by rb07

  • Resolution fixed deleted
  • Status changed from closed to reopened

The change leaves UTP_GetMicroseconds() undefined in the library for Windows.

comment:6 follow-up: Changed 8 years ago by jordan

rb07, does r12267 solve that problem?

comment:7 in reply to: ↑ 6 Changed 8 years ago by rb07

Replying to jordan:

rb07, does r12267 solve that problem?

No, there are now 2 UTP_GetMilliseconds() :

  CXX    utp_utils.o
utp_utils.cpp:9:0: warning: "WIN32_LEAN_AND_MEAN" redefined [enabled by default]
<command-line>:0:0: note: this is the location of the previous definition
utp_utils.cpp: In function 'uint32 UTP_GetMilliseconds()':
utp_utils.cpp:167:8: error: redefinition of 'uint32 UTP_GetMilliseconds()'
utp_utils.cpp:36:8: error: 'uint32 UTP_GetMilliseconds()' previously defined here

I guess the first one is redundant...

comment:8 follow-up: Changed 8 years ago by jordan

hmm, how about r12269? :)

comment:9 in reply to: ↑ 8 Changed 8 years ago by rb07

Replying to jordan:

hmm, how about r12269? :)

Yes.

comment:10 Changed 8 years ago by jordan

  • Resolution set to fixed
  • Status changed from reopened to closed

if I'm reading the previous comment correctly, this solves the issue that got this ticket reopened in comment:5 ...

comment:11 Changed 8 years ago by livings124

  • Milestone 2.30 deleted

comment:12 Changed 8 years ago by jch

I'm going to push r12269 upstream. What git id should I use for the patch author? I need something that looks like

Bill Gates <bill@…>

i.e. something that looks like a full name followed with something that looks like an e-mail. (It doesn't need to be a real e-mail, but it's better if it is.)

--jch

comment:13 Changed 8 years ago by jch

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening to make sure people see my request.

-- jch

comment:14 Changed 8 years ago by jordan

  • Resolution set to fixed
  • Status changed from reopened to closed

jch, my github account is jordanl <jordan@transmissionbt.com>

Note: See TracTickets for help on using tickets.