search

ddvk / remarkable2-framebuffer

remarkable2 framebuffer reversing
MIT License
272 stars 22 forks source link

Update timespec overflow implementation #81

Closed LinusCDE closed 2 years ago

LinusCDE commented 2 years ago

This was raised by @fenollp when reviewing the rust-impl of this client.

https://github.com/canselcik/libremarkable/pull/78#discussion_r748864139

The code wouldn't work properly if SEM_WAIT_TIMEOUT would be greater than one second, since only one overflow is accounted for with an "if".

Using modulo here instead of while so no branching is needed and it would scale linearly not not require one loop per overflown second.

(I didn't use 1e9 since it's a double and I'm not sure if it would be perfectly accurate in C++, feel free to change if you don't mind.)

LinusCDE commented 2 years ago

Never mind, changed it back to using 1e9 with casting.

I'm a bit confused as of which impl is used. If glibc/frida's is used over the default one, it should be fine.

screenshot4328

The default one according to my (x86_64) linux headers seems like it would be 32 bit on

grafik

This would still cause a native overflow issue for delays bigger than ~4.2s as this overflows the 32 bit long on the device.

grafik

We will probably not run into an issue in the foreseeable future, but it should still be noted as a possible problem in the implementation.

LinusCDE commented 2 years ago

Edit: The glib/frida impl (glong) actually uses a normal long as well (32 bit) and hence would cause the problem as well.

LinusCDE commented 2 years ago

(The issue could also affect the rust impl as well since it seems that c_long is used for tv_nsec on 32 bit non x86_64 systems)

grafik

alistair23 commented 2 years ago

tv_nsec should always be a long, see https://linux.die.net/man/3/clock_gettime

The x32 ABI is a strange exception that other architectures don't use.

alistair23 commented 2 years ago

Also, it's probably just better to use timeradd to add these: https://man7.org/linux/man-pages/man3/timeradd.3.html

raisjn commented 2 years ago

@alistair23: side question: for the rM1, we think that the wacom driver is drawing power while the device is in deep sleep. would you happen to know how to disable / send the wacom driver (on the i2c bus) a suspend signal or otherwise disable it? (or how to confirm if it is drawing power?) thanks!

alistair23 commented 2 years ago

I'm not sure sorry

LinusCDE commented 2 years ago

Also, it's probably just better to use timeradd to add these: https://man7.org/linux/man-pages/man3/timeradd.3.html

Sorry for the late response. This seems probably like the best way. But the current solution should be fine for now. Goes to demonstrate how many levels of better exist for such seemingly a simple task.