eBay / eBay/HomeStore

VirtualDev::alloc_blk nblks 16bit overflow can possibly cause silent data corruption

Open
#560 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
C++
Stars
29
Forks
30
Avg merge
2d 4h
Merged PRs (30d)
6

Description

Problem Statement:

VirtualDev::alloc_blk takes 16bit nblks, however existing caller in homestore take 32bit data size and pass 32bit nblks to vdev layer which will cause overflow when the data size is larger than 256MB, it will cause corruption (worst case data mismatch) when data size being written is larger than 256MB.

Meta Svc layer has protection when the overflow happens, e.g. it can't match the input context_sz with the nblks matched sizes and will hit release assert, however this is still will cause crash.

Code pointer:
repl_req_ctx::alloc_local_blks
--> auto status = data_service().alloc_blks(sisl::round_up(uint32_cast(data_size), // <<< overflow
BlkDataService::alloc_blks
--> return m_vdev->alloc_blks(nblks, hints, out_blkids); // <<< overflow
MetaBlkService::alloc_meta_blks
--> const auto ret = m_sb_vdev->alloc_blks(nblks, blk_alloc_hints{}, bids); // <<< overflow

The underlying alloc_blks when overflow happens, if we go deep into the actual blk allocator (both the bitmap and append blk allocator), it will return SUCCESS with bids.size() equls to zero. There is no bug in the underlying blkallocator, the point is there is no assert in them also to detect the input nblks is actual 0, it will just return SUCCESS with nothing allocated, when this happens, silent data corruption will happen.

The fix needs to be done at vdev layer.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at VirtualDev::alloc_blk and trace the named callers repl_req_ctx::alloc_local_blks, BlkDataService::alloc_blks, and MetaBlkService::alloc_meta_blks. Inspect how the vdev layer handles nblks above its 16-bit range and how the bitmap and append allocators respond. Done means oversized requests are detected at the vdev boundary instead of succeeding with no block IDs and silently corrupting data; add regression coverage if the repository’s test structure identifies a suitable location.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.