fat32: print the volume label instead of the literal "%.11s" - #127
Merged
Conversation
vformat has no precision support. It parses flags, then width, then the
length modifier, then the conversion -- there is no '.' handling anywhere,
so "%.11s" fell through to the conversion switch's default arm, which emits
'%' plus the offending character and then resumes at "11s". Every successful
FAT32 mount has therefore printed:
[FAT32] Volume label: %.11s
This is the same class as the %zu bug fixed in 88258c0, and the comment in
kprintf.c already documents the mechanism. It is milder here only by luck:
the unrecognized specifier does not consume its vararg, but the label is the
last argument, so nothing shifts. A precision specifier anywhere before
another argument would corrupt the rest of the line.
-Wformat cannot catch it. The format string is valid C and the argument
genuinely matches %.11s -- it is TinyOS's own parser that does not implement
it, which is exactly why this shipped.
boot_sector.volume_label is a non-NUL-terminated uint8_t[11], so plain %s
would read past the field into the following boot-sector members. Copy to a
12-byte local and terminate instead.
Verified on a real mount (mformat's default label, padded to 11):
[FAT32] Volume label: NO NAME
[FAT32] Mount successful [OK]
Build stays warning-clean under -Werror.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CEkhAhgTxbE5TgifyYf8v4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Every successful FAT32 mount printed the format specifier rather than the label:
Now:
Why it happened
vformatparses flags → width → length modifier → conversion. There is no.handling at any stage, so%.11sfalls through to the conversion switch'sdefaultarm, which emits'%'plus the character it stopped on and then resumes at11s.This is the same class as the
%zubug fixed in 88258c0, whose comment inkprintf.calready spells out the mechanism. It is milder here only by luck: an unrecognized specifier does not consume its vararg, but the label is the last argument, so nothing shifts. The same specifier placed before another argument would corrupt everything after it.-Wformatcannot catch this, even with the format attribute added in the earlier sweep — the format string is valid C and the argument really does match%.11s. It is TinyOS's own parser that doesn't implement precision, which is why it shipped and stayed.The fix
boot_sector.volume_labelis a non-NUL-terminateduint8_t[11], so plain%swould read past the field into the following boot-sector members. Copy into a 12-byte local and terminate.Adding precision support to
vformatwas the alternative. Not done here: one live site tree-wide (user.c:619is commented out,shell_system.c:198is a comment about this same bug class), so the parser change carries more risk than it removes.Verification
make -j8 kernel.elfclean under-Werror.git diff --statand--ignore-all-spaceboth report 7 insertions, so no whole-file line-ending flip.Found while investigating #126; unrelated to it and split out deliberately.