pycom / pycom-micropython-sigfox

A fork of MicroPython with the ESP32 port customized to run on Pycom's IoT multi-network modules.
MIT License
199 stars 167 forks source link

Integer overflow in filesystem stat implementation #409

Closed amotl closed 4 years ago

amotl commented 4 years ago

Dear Pycom team,

while investigating an issue why rshell would upload our files over and over again despite its rsync implementation should compensate for that (https://github.com/dhylands/rshell/issues/124), we just found an issue when looking at mtime timestamps of the files created on the device.

We found that there might be an integer overflow within the virtual filesystem implementation keeping track of filesystem metadata.

This example says a thousand words:

>>> import time; f = open('/flash/test', 'w'); f.write('hello'); f.close(); time.time() - os.stat('/flash/test')[7]
2147483648

It looks like only newer firmware releases are affected. The bug apparently started appearing in 1.20.0.rc0 and is still visible in 1.20.2rc3-pybytes. However, it is not present in 1.18.2.r7 and 1.18.3-pybytes. It looks like it is independent of FatFS vs. LittleFS.

2147483648 is just 2^31 or INT_MAX + 1 or abs(INT_MIN). The correct outcome should be zero (0), or sometimes 1 - dependent on how fast the oneliner is executing.

Thanks already for looking into this.

With kind regards, Andreas.

cc @husigeza

robert-hh commented 4 years ago

Bug Fix: File esp32/extmod/vfs_fat.c, Zeilen 368-370 File esp32/littlefs/vfs_littlefs.c, Zeilen 689-691 Replace MP_OBJ_NEW_SMALL_INT by mp_obj_new_int_from_uint

P.S.: I'm not going to send a PR for that, since these seem to end at the office in charge for that at Alpha Centauri.

amotl commented 4 years ago

That was quick. I will submit a PR later. Thank you so much, Robert!

amotl commented 4 years ago

We just verified through https://github.com/dhylands/rshell/issues/124#issuecomment-591205648 that changing these bits brings much of joy, especially when accompanied by https://github.com/dhylands/rshell/pull/125.

Thanks again @robert-hh!

amotl commented 4 years ago

Thanks for merging this!