Add stronger checks for offset in seek

[lucic71]

Previous checks were trying to validate that file->pos does not become negative. However, the method used for checking this contains possible undefined behaivor (UB) because of the signed integer overflow.

This commit adds stronger checks for offset calculation:

• make sure that ((lfs_soff_t) file->pos + off) is never < 0. Instead of using signed addition to check that (which can possibly lead to UB), use signed comparison: off < 0 && (lfs_soff_t) file->pos < -off. A special check of off against INT32_MIN is added to make sure that -off does not get transformed into -INT32_MIN, which is as well UB.
• make sure that unsigned overflow does not occur in file->pos + (lfs_off_t) off.

Thoughts:

• the lseek manual mandates an EOVERFLOW when the new offset cannot be represened in the offset type. I wonder if we want to return that instead of INVAL when an unsigned overflow occurs.

[geky-bot]

Tests passed ✓, Code: 17136 B (+0.4%), Stack: 1440 B (+0.0%), Structs: 812 B (+0.0%)

	Code	Stack	Structs		Coverage
Default	17136 B (+0.4%)	1440 B (+0.0%)	812 B (+0.0%)	Lines	2397/2578 lines (-0.0%)
Readonly	6270 B (+1.2%)	448 B (+0.0%)	812 B (+0.0%)	Branches	1253/1594 branches (+0.0%)
Threadsafe	17996 B (+0.4%)	1440 B (+0.0%)	820 B (+0.0%)		Benchmarks
Multiversion	17196 B (+0.4%)	1440 B (+0.0%)	816 B (+0.0%)	Readed	29369693876 B (+0.0%)
Migrate	18832 B (+0.4%)	1744 B (+0.0%)	816 B (+0.0%)	Proged	1482874766 B (+0.0%)
Error-asserts	17820 B (+0.4%)	1432 B (+0.0%)	812 B (+0.0%)	Erased	1568888832 B (+0.0%)

[geky]

Hi @lucic71, thanks for creating a PR, sorry about the late response.

These checks were originally implemented before v2.9, in an attempt to provide limited support for 32-bit file sizes. But this idea has since been dropped due to API issues, with littlefs now strictly using 31-bit file sizes.

Which makes me wonder, is it possible to simplify all of this by just doing unsigned arithmetic everywhere and checking that the result is in the 31-bit range?

We probably don't need to worry about ever needing full 32-bit support as it would make more sense to provide a 63-bit variant at that point.

[geky]

the lseek manual mandates an EOVERFLOW when the new offset cannot be represened in the offset type. I wonder if we want to return that instead of INVAL when an unsigned overflow occurs.

This is an interesting question. Playing around with the API on 64-bit Linux I seem to only be able to get EINVAL as a result.

Maybe EOVERFLOW is intended for when the result doesn't fit in an off_t, but does fit in the filesystem's theoretical max file size? Will need to investigate.

[geky]

Hi @lucic71, I went ahead and pushed up an alternative implementation relying on 31-bit file sizes here: #1027

Let me know if you see any issues with it, and thanks for the original fix.