Fix available space check for 32-bit systems

[eu-siemann]

The issue was reported by a customer. This function:

aktualizr/src/libaktualizr/storage/sqlstorage.cc

Line 1633 in 3c09519

bool SQLStorage::checkAvailableDiskSpace(const uint64_t required_bytes) const {

gives incorrect results on 32-bit systems.

The root cause is that both f_bsize and f_bavail members of statvfs struct are 4 bytes long. Hence, the result of this expression

aktualizr/src/libaktualizr/storage/sqlstorage.cc

Line 1640 in 3c09519

const uint64_t available_bytes = (stvfsbuf.f_bsize * stvfsbuf.f_bavail);

is also 4 byte long and overflows on 4GB.

This fix makes aktualizr use 64-bit filesystem interface. Member f_bavail becomes 8 bytes long and the expression evaluates correctly.
This has no effect on 64-bit architectures.

[pattivacek]

Looks fine, but how gcc-specific is this? Looks like it's compiler-dependent, but most compilers support it?

[lbonn]

Defining the macro is probably a good idea for preventing future issues but I would also add some manual casts in the parts we know are problematic.

Here, casting stvfsbuf.f_bsize to uint64_t should not hurt.

[eu-siemann]

This has nothing to do with gcc, but it rather depends on C standard library implementation. So far I can see that it's implemented in the same way in glibc and ulibc.

[codecov-io]

Codecov Report

Merging #1588 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1588      +/-   ##
==========================================
- Coverage   82.35%   82.33%   -0.02%     
==========================================
  Files         189      189              
  Lines       11851    11851              
==========================================
- Hits         9760     9758       -2     
- Misses       2091     2093       +2

Impacted Files	Coverage Δ
src/libaktualizr/storage/sqlstorage.cc	77.98% <100%> (+0.19%)	⬆️
src/libaktualizr/storage/sqlstorage_base.cc	79.05% <0%> (-2.03%)	⬇️
src/libaktualizr/package_manager/ostreemanager.cc	79.04% <0%> (-0.74%)	⬇️
src/libaktualizr/primary/sotauptaneclient.cc	90.78% <0%> (+0.12%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c09519...0790398. Read the comment docs.

[kbushgit]

This fix makes aktualizr use 64-bit filesystem interface. Member f_bavail becomes 8 bytes long

I've tried simple example and I do not see this transformation, or my test is incorrect?

sudo apt-get install g++-multilib

#include <sys/statvfs.h>
#include <iostream>

int main() {
  struct statvfs stvfsbuf {};
  const int stat_res = statvfs("/", &stvfsbuf);

  const auto available_bytes = (stvfsbuf.f_bsize * stvfsbuf.f_bavail);

  std::cout << "sizeof stvfsbuf.f_bsize -> " << sizeof(stvfsbuf.f_bsize) << std::endl;
  std::cout << "sizeof stvfsbuf.f_bsize -> " << sizeof(stvfsbuf.f_bsize) << std::endl;
  std::cout << "sizeof avaiulable -> " << sizeof(available_bytes) << std::endl;

  return 0;
}

g++ -m32 -std=c++11 main.cc

result:

sizeof stvfsbuf.f_bsize -> 4
sizeof stvfsbuf.f_bsize -> 4
sizeof avaiulable -> 4

with flag:

g++ -m32 -D_FILE_OFFSET_BITS=64 -std=c++11 main.cc

result:

sizeof stvfsbuf.f_bsize -> 4
sizeof stvfsbuf.f_bsize -> 4
sizeof avaiulable -> 8

[lbonn]

@kbushgit there is indeed an error in your program, it prints sizeof(stvfsbuf.f_bsize) twice.

After the fix:

$ ./statvfs
sizeof stvfsbuf.f_bsize -> 4
sizeof stvfsbuf.f_bavail -> 8
sizeof available -> 8

[pattivacek]

Can we take the idea behind Kosta's test app and turn it into a proper unit test?

[eu-siemann]

Defining the macro is probably a good idea for preventing future issues but I would also add some manual casts in the parts we know are problematic.

Here, casting stvfsbuf.f_bsize to uint64_t should not hurt.

I think you are right. I wanted to avoid explicit castings, but we still might end up with u32 * u32, which is wrong.

[eu-siemann]

Can we take the idea behind Kosta's test app and turn it into a proper unit test?

A proper way to do it would be to have something similar to this : https://github.com/libarchive/libarchive/blob/master/build/cmake/CheckFileOffsetBits.cmake
But I don't think it's worth an effort, tbh.