otp icon indicating copy to clipboard operation
otp copied to clipboard

Possible value overflow in `erts_time_unit_conversion`

Open cocoa-xu opened this issue 1 year ago • 2 comments

Describe the bug The result of the converted time can exceed the maximum possible value of uint64_t.

To Reproduce For an instance, on my ARM64 Windows machine (this issue can happen on any machine that meets the following conditions), it doesn't have the macro ERTS_COMPILE_TIME_MONOTONIC_TIME_UNIT defined, and the ERTS_MONOTONIC_TIME_UNIT is initialised to 24000000 in erts_init_sys_time_sup

https://github.com/erlang/otp/blob/51f7331c500d8409645dc023c92b689264b7e6b1/erts/emulator/beam/erl_time_sup.c#L917-L925

The converted value using nano seconds as the time unit can overflow in

https://github.com/erlang/otp/blob/51f7331c500d8409645dc023c92b689264b7e6b1/erts/emulator/beam/erl_time_sup.c#L1019-L1022

Let's say the abs_native_offset is 576460752312000000, the above code is equivalent to

	erts_time_sup__.r.o.start_offset.nsec = (ErtsMonotonicTime)
	erts_time_unit_conversion((Uint64) 576460752312000000,
				  (Uint32) 24000000,
				  (Uint32) 1000*1000*1000);

The erts_time_unit_conversion is expecting to return 24019198012999999488 (btw it's impossible because this value cannot be representable using a uint64_t)

> trunc(576460752312000000 / 24000000 * 1000*1000*1000).
24019198012999999488

In this case, in erts_time_unit_conversion, high will be 5592405333 (16#1_4D55_5555) before left shift 32bits, which will overflow after the bit shift operation.

https://github.com/erlang/otp/blob/51f7331c500d8409645dc023c92b689264b7e6b1/erts/emulator/beam/erl_time.h#L210-L211

Expected behavior erts_time_unit_conversion can handle large integers without overflow.

Affected versions Tested on OTP 27.0-rc1. But this issue should exist in all OTP versions.

Additional context poc.c

cocoa-xu avatar Feb 25 '24 12:02 cocoa-xu

Thanks for your report! As far as I can tell the result is correct: it's supposed to calculate the converted time modulo 2 ** 64. This should not cause issues beyond a certain optimization going away, so you might have run into a bug where some code didn't expect this.

How did you discover this? Did something crash?

jhogberg avatar Mar 13 '24 13:03 jhogberg

How did you discover this? Did something crash?

I was trying to build Erlang/OTP for ARM64 Windows (#8142). And I found that the testcase, tests.emulator_test.time_SUITE.time_unit_conversion, failed because this value will overflow when converting time using nanoseconds as the time unit.

Screenshot 2024-03-13 at 21 45 55

cocoa-xu avatar Mar 13 '24 13:03 cocoa-xu

Thanks again for your report, we merged a fix three weeks ago but I forgot to make a note of it here. :)

jhogberg avatar Apr 30 '24 11:04 jhogberg

Thanks again for your report, we merged a fix three weeks ago but I forgot to make a note of it here. :)

Thank you very much for the update! ;)

cocoa-xu avatar Apr 30 '24 12:04 cocoa-xu