From 77a27462d53850f32143e1a8e68fcd087d355e92 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Sun, 30 Aug 2026 17:31:52 +0200 Subject: [PATCH] kernel: Avoid out-of-range shifts in CopyBits CopyBits and its helpers shift by amounts derived from , and the alignment difference between the two blocks. Several of those can reach BIPEB, where `1 << BIPEB` and `x >> BIPEB` are undefined: * After a partial first destination word is filled, can be left equal to BIPEB rather than wrapping to the next source word. MaskForCopyBits then evaluates `(UInt)1 << BIPEB`. * In the main loop equal to 0 makes the second half of the word assembly shift by `BIPEB - 0`. * CopyInWord shifts by . The callers do keep it in range, but only indirectly: in the `tobit + BIPEB - frombit` case it takes the branch condition `frombit + tailbits > BIPEB` to show frombit > tobit. Normalise to the start of the next word, make MaskForCopyBits total so an out-of-range bound yields an empty mask, special-case the aligned main loop, and bound the shift in CopyInWord directly. An exhaustive sweep over frombit, tobit in [0,BIPEB) and nbits in [1,5*BIPEB] hits the undefined shift in 1953 of 1310720 cases, and gives identical results before and after: in every one of those cases the mask being computed is dead, because no whole words remain to copy. So this changes no observable behaviour on current compilers. The guards also do a second job. They are what lets a static analyzer establish the bounds: without them clang's core.BitwiseShift reports "Left shift overflows the capacity of 'UInt'" for paths reaching here from blister.c and vecgf2.c. Please keep them even where a local reading suggests the branch is dead. Co-authored-by: Claude Opus 5 --- src/bits_intern.h | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/src/bits_intern.h b/src/bits_intern.h index 4d28e9fe6b..4cd61f5f57 100644 --- a/src/bits_intern.h +++ b/src/bits_intern.h @@ -38,7 +38,13 @@ // constructs a mask that selects bits to inclusive of a UInt static inline UInt MaskForCopyBits(UInt from, UInt to) { - return ((to == BIPEB - 1) ? 0 : ((UInt)1 << (to + 1))) - ((UInt)1 << from); + UInt high = 0; + UInt low = 0; + if (to < BIPEB - 1) + high = (UInt)1 << (to + 1); + if (from < BIPEB) + low = (UInt)1 << from; + return high - low; } /* copies a block of bits from the UInt to the one pointed at @@ -52,10 +58,18 @@ CopyInWord(UInt * to, UInt startbit, UInt endbit, UInt from, Int shift) { UInt m = MaskForCopyBits(startbit + shift, endbit + shift); *to &= ~m; - if (shift >= 0) - *to |= ((from << shift) & m); - else - *to |= ((from >> -shift) & m); + if (shift >= 0) { + UInt lshift = (UInt)shift; + if (lshift >= BIPEB) + return; + *to |= ((from << lshift) & m); + } + else { + UInt rshift = (UInt)(-shift); + if (rshift >= BIPEB) + return; + *to |= ((from >> rshift) & m); + } } @@ -72,6 +86,8 @@ static ALWAYS_INLINE void CopyBits(const UInt * fromblock, return; GAP_ASSERT(frombit < BIPEB); GAP_ASSERT(tobit < BIPEB); + if (frombit >= BIPEB || tobit >= BIPEB) + return; /* If the alignment of the two data blocks matches, things are relatively * easy */ @@ -126,13 +142,24 @@ static ALWAYS_INLINE void CopyBits(const UInt * fromblock, toblock++; nbits -= tailbits; tobit = 0; + if (frombit == BIPEB) { + frombit = 0; + fromblock++; + } } // Main loop for long copies fills whole blocks of destination + if (frombit >= BIPEB) + return; UInt m1 = MaskForCopyBits(frombit, BIPEB - 1); while (nbits >= BIPEB) { - x = (*fromblock++ & m1) >> frombit; - x |= (*fromblock & ~m1) << (BIPEB - frombit); + if (frombit == 0) { + x = *fromblock++; + } + else { + x = (*fromblock++ & m1) >> frombit; + x |= (*fromblock & ~m1) << (BIPEB - frombit); + } *toblock++ = x; nbits -= BIPEB; }