Skip to content

libdevs: make the safe device bitmap word unsigned - #288

Merged
AltraMayor merged 1 commit into
AltraMayor:masterfrom
mminkus:fix-sdev-bitmap-signed-shift
Sep 16, 2026
Merged

AltraMayor merged 1 commit into
AltraMayor:masterfrom
mminkus:fix-sdev-bitmap-signed-shift

Conversation

@mminkus

@mminkus mminkus commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

SDEV_BITMAP_WORD is long, so sdev_is_block_saved() and sdev_mark_blocks()
evaluate (long)1 << 63 whenever a block position is congruent to 63 modulo 64.
Shifting into the sign bit of a signed type is undefined behaviour.

In practice gcc produces the intended bit pattern today, so the bitmap happens to
work. It is still UB, and the bitmap is what the safe device uses to decide
whether a block has already been saved, so a wrong answer there would either lose
an original block or consume the saved block budget twice.

Reproducing

Build with UBSan and probe a simulated device:

make build/f3probe \
  CFLAGS="-std=c17 -Wall -Wextra -pedantic -MMD -ggdb -fsanitize=undefined" \
  LDFLAGS="-fsanitize=undefined"

./build/f3probe --debug --debug-real-size=264241152 \
  --debug-fake-size=67108864000 --debug-wrap=36 /tmp/sim
src/libdevs.c:1158:32: runtime error: left shift of 1 by 63 places cannot be
    represented in type 'long int'
src/libdevs.c:1170:51: runtime error: left shift of 1 by 63 places cannot be
    represented in type 'long int'

Verifying the fix

I swept 63 simulated geometries, varying real size, announced size and wrap.
Every run reported the shift before this change; none do after it, and every
probed size is identical either way, so this is a pure UB fix with no behaviour
change.

Found while probing a counterfeit card on aarch64, Debian 13, gcc 14.2.

SDEV_BITMAP_WORD is long, so sdev_is_block_saved() and sdev_mark_blocks()
evaluate (long)1 << 63 whenever a block position is congruent to 63 modulo
64. Shifting into the sign bit of a signed type is undefined behaviour.

gcc currently produces the intended bit pattern, so the bitmap happens to
work, but the behaviour is not guaranteed and the bitmap is what the safe
device uses to decide whether a block has already been saved. A wrong
answer there would either lose an original block or consume the saved
block budget twice.

Building with -fsanitize=undefined and probing a simulated device reports
the shift on every run:

  src/libdevs.c:1158:32: runtime error: left shift of 1 by 63 places
      cannot be represented in type 'long int'
  src/libdevs.c:1170:51: runtime error: left shift of 1 by 63 places
      cannot be represented in type 'long int'

Across 63 simulated geometries, every run flagged it. With the word made
unsigned, none do, and every probed size is unchanged.
@AltraMayor
AltraMayor merged commit 722521f into AltraMayor:master Sep 16, 2026
26 checks passed
@AltraMayor

Copy link
Copy Markdown
Owner

Thank you for this review, @mminkus.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants