badblocks: fix infinite loop and validate sector range/shift#5
Open
blktests-ci-block-trial[bot] wants to merge 2 commits into
Open
badblocks: fix infinite loop and validate sector range/shift#5blktests-ci-block-trial[bot] wants to merge 2 commits into
blktests-ci-block-trial[bot] wants to merge 2 commits into
Conversation
rounddown() and roundup() do not modify their first argument in place; they return the rounded value. _badblocks_set(), _badblocks_clear() and badblocks_check() were calling them as bare statements and discarding the result, so 's' (and 'next'/ 'target') were never actually rounded. Depending on the caller's alignment this can leave 'sectors' unchanged or, in the reported case, produce a range whose end never advances, causing _badblocks_check()/badblocks_check() to loop with a non-advancing cursor and stall the CPU (RCU stall) when called through the nvdimm ioctl path via nvdimm_clear_badblocks_region(). rounddown()/roundup() also do division/modulo on the sector_t (u64) operand, which requires libgcc helpers (__aeabi_uldivmod, __umoddi3) that are not linked into the kernel on 32-bit builds, breaking the build on arm/i386 (reported by kernel test robot). Switch to round_down()/round_up() (include/linux/math.h), which are mask-based, assign their result back to the variable being rounded, and require no 64-bit division, fixing both the non-rounding bug and the 32-bit build breakage. Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202604301231.IpPh4AiH-lkp@intel.com/ Fixes: aa511ff ("badblocks: switch to the improved badblock handling code") Cc: stable@vger.kernel.org Signed-off-by: Ramesh Adhikari <adhikari.resume@gmail.com>
_badblocks_set(), _badblocks_clear() and badblocks_check() round the caller-supplied [s, s+sectors) range to the current bb->shift block size before touching the bad block table. That rounding was not defensive against a few cases: - s + sectors can overflow sector_t (u64), wrapping the range end before it is ever compared against s. - bb->shift is a plain 'int' field, populated in one case (drivers/md/md.c, from the on-disk superblock's bblog_shift) straight from an unvalidated byte with no upper bound. Shifting by an amount >= the width of the shifted type is undefined behaviour in C; "1 << bb->shift" was shifting an int literal, so this was already undefined for bb->shift >= 32, let alone the full 0-255 range bblog_shift allows. - round_up()/round_down() rounding a value near ULLONG_MAX can itself wrap back to a small value, so even with a valid shift the rounded end of the range could end up smaller than the rounded start, silently turning a small range into a huge one (in _badblocks_clear()/badblocks_check(), which round the end up) or losing the range entirely. Add an explicit s+sectors overflow check, reject any bb->shift that is too large to shift a sector_t by, cast the shift operand to sector_t so the shift itself is never performed on a 32-bit int, and detect post-rounding wrap by comparing the rounded result back against the pre-rounding value. Suggested-by: Coly Li <colyli@fygo.io> Fixes: aa511ff ("badblocks: switch to the improved badblock handling code") Cc: stable@vger.kernel.org Signed-off-by: Ramesh Adhikari <adhikari.resume@gmail.com>
Author
|
Upstream branch: a635d67 |
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.
Pull request for series with
subject: badblocks: fix infinite loop and validate sector range/shift
version: 6
url: http://redsun45:8000/project/linux-block/list/?series=274