Skip to content

Commit

Permalink
Merge bitcoin#27235: Avoid integer overflow in CheckDiskSpace
Browse files Browse the repository at this point in the history
05eeba2 [test] Add manual prune startup test case (dergoegge)
4517419 [util] Avoid integer overflow in CheckDiskSpace (dergoegge)

Pull request description:

  Starting a fresh node with `-prune=1` causes an integer overflow to happen in `CheckDiskSpace` ([here](https://github.com/bitcoin/bitcoin/blob/f7bdcfc83f5753349018be3b5a663c8923d1a5eb/src/init.cpp#L1633-L1648)) because `nPruneTarget` is to the max `uint64_t` value.
  ```
   node1 stderr util/system.cpp:138:51: runtime error: unsigned integer overflow: 52428800 + 18446744073709551615 cannot be represented in type 'unsigned long'
      #0 0x564a482b5088 in CheckDiskSpace(fs::path const&, unsigned long) src/./src/util/system.cpp:138:51
      #1 0x564a4728dc59 in AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/./src/init.cpp:1639:14
      #2 0x564a47256e6a in AppInit(node::NodeContext&, int, char**) src/./src/bitcoind.cpp:221:43
      #3 0x564a47256087 in main src/./src/bitcoind.cpp:265:13
      #4 0x7fcb7cbffd8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)
      #5 0x7fcb7cbffe3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)
      #6 0x564a471957f4 in _start (/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/bitcoind+0xca07f4) (BuildId: 035cb22302d37317a630900a15a26ecb326d395c)
  SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow util/system.cpp:138:51 in
  ```

  I think side stepping the overflow for this specific case, is better than adding an exception to the UB suppresions file.

ACKs for top commit:
  MarcoFalke:
    ACK 05eeba2 🥝
  john-moffett:
    ACK 05eeba2

Tree-SHA512: 1d8e6bcb49818139f04b5ab2cbef7f9b422bf0c38a804cd532b6bd0ba4c4fd07f959ba977e59896343f213086c8ecc48180f50d006638dc84649c66ec379d58a
  • Loading branch information
glozow committed Mar 13, 2023
2 parents 73a9892 + 05eeba2 commit f50fb17
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1631,10 +1631,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)

// On first startup, warn on low block storage space
if (!fReindex && !fReindexChainState && chain_active_height <= 1) {
uint64_t assumed_chain_bytes{chainparams.AssumedBlockchainSize() * 1024 * 1024 * 1024};
uint64_t additional_bytes_needed{
chainman.m_blockman.IsPruneMode() ?
chainman.m_blockman.GetPruneTarget() :
chainparams.AssumedBlockchainSize() * 1024 * 1024 * 1024};
std::min(chainman.m_blockman.GetPruneTarget(), assumed_chain_bytes) :
assumed_chain_bytes};

if (!CheckDiskSpace(args.GetBlocksDirPath(), additional_bytes_needed)) {
InitWarning(strprintf(_(
Expand Down
8 changes: 8 additions & 0 deletions test/functional/rpc_blockchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ def set_test_params(self):

def run_test(self):
self.wallet = MiniWallet(self.nodes[0])
self._test_prune_disk_space()
self.mine_chain()
self._test_max_future_block_time()
self.restart_node(
Expand Down Expand Up @@ -100,6 +101,13 @@ def mine_chain(self):
self.generate(self.wallet, 1)
assert_equal(self.nodes[0].getblockchaininfo()['blocks'], HEIGHT)

def _test_prune_disk_space(self):
self.log.info("Test that a manually pruned node does not run into "
"integer overflow on first start up")
self.restart_node(0, extra_args=["-prune=1"])
self.log.info("Avoid warning when assumed chain size is enough")
self.restart_node(0, extra_args=["-prune=123456789"])

def _test_max_future_block_time(self):
self.stop_node(0)
self.log.info("A block tip of more than MAX_FUTURE_BLOCK_TIME in the future raises an error")
Expand Down

0 comments on commit f50fb17

Please sign in to comment.