* [PATCH 0/1] Fix random.c UB on int overflow
@ 2026-10-06 13:51 Lukas Herbolt
2026-10-06 13:51 ` [PATCH 1/1] lib/random.c fix undefined behavior on signed integer overflow Lukas Herbolt
0 siblings, 1 reply; 5+ messages in thread
From: Lukas Herbolt @ 2026-10-06 13:51 UTC (permalink / raw)
To: zlang; +Cc: fstests, linux-xfs, linux-ext4, Lukas Herbolt
While building xfstests with default rpm CFLAGS/LDFLAGS, I noticed
the generic/007 fails with:
--- /opt/xfstests-dev/tests/generic/007.out 2026-10-06 14:08:02.556220624 +0200
+++ /opt/xfstests-dev/results//generic/007.out.bad 2026-10-06 14:40:01.667593670 +0200
@@ -14,9 +14,9 @@
.........................................................................
.........................................................................
....................................................
-creates: 18736 OK, 18802 EEXIST ( 37538 total, 50% EEXIST)
-removes: 18675 OK, 19927 ENOENT ( 38602 total, 51% ENOENT)
-lookups: 12000 OK, 11860 ENOENT ( 23860 total, 49% ENOENT)
-total : 49411 OK, 50589 w/error (100000 total, 50% w/error)
+creates: 18745 OK, 19086 EEXIST ( 37831 total, 50% EEXIST)
+removes: 18685 OK, 19864 ENOENT ( 38549 total, 51% ENOENT)
+lookups: 12065 OK, 11555 ENOENT ( 23620 total, 48% ENOENT)
+total : 49495 OK, 50505 w/error (100000 total, 50% w/error)
-cleanup: 61 removes
+cleanup: 60 removes
This happens because because the lib/random.c has optimized out the negative
branch check as overflow on signed int is undefined behavior.
if (it <= 0)
it = (it + it) ^ MASK;
else
it = it + it;
Changing the if-else to single if based on the original `it` value prevents the
compiler from optimizing out the code.
int apply_mask = (it <= 0);
it = (unsigned)it << 1;
if (apply_mask)
it ^= MASK;
Steps to reproduce:
dnf install redhat-rpm-config -y
./configure CFLAGS="$(rpm --eval '%{optflags}')" \
LDFLAGS="$(rpm --eval '%{__global_ldflags}')"
./check generic/007
The random.c is used in:
ext4/052
generic/007
generic/010
generic/011
generic/014
generic/094
generic/225
generic/311
generic/676
xfs/008
xfs/188
and I did not noticed any regression with this change and the only one
failing with the original implementation is the generic/007.
Lukas Herbolt (1):
lib/random.c fix undefined behavior on signed integer overflow.
lib/random.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
--
2.55.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] lib/random.c fix undefined behavior on signed integer overflow.
2026-10-06 13:51 [PATCH 0/1] Fix random.c UB on int overflow Lukas Herbolt
@ 2026-10-06 13:51 ` Lukas Herbolt
2026-10-07 17:00 ` Darrick J. Wong
0 siblings, 1 reply; 5+ messages in thread
From: Lukas Herbolt @ 2026-10-06 13:51 UTC (permalink / raw)
To: zlang; +Cc: fstests, linux-xfs, linux-ext4, Lukas Herbolt
The random.c now expects that signed integer to overflow and
triggers the ^MASK branch. But signed int overflow is undefined
behavior and GCC can optimize this branch out with certain
CFLAGS/LDFLAGS.
Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
---
lib/random.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/lib/random.c b/lib/random.c
index d5c81be817c8..f767ed1d1337 100644
--- a/lib/random.c
+++ b/lib/random.c
@@ -186,14 +186,15 @@ _random (int32_t is [2])
int32_t
_irandm (int32_t is [2])
{
- int32_t it, leh, nit;
-
+ int32_t it, leh, nit, apply_mask;
it = is [0];
leh = is [1];
- if (it <= 0)
- it = (it + it) ^ MASK;
- else
- it = it + it;
+
+/* evaluate on original value — no UB here */
+ apply_mask = (it <= 0);
+/* double via unsigned shift — well-defined */
+ it = (unsigned)it << 1;
+ if (apply_mask) it ^= MASK;
nit = it - 1;
/* to ensure all-ones pattern omitted */
leh = leh * mt[nit & 127] + nit;
--
2.55.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] lib/random.c fix undefined behavior on signed integer overflow.
2026-10-06 13:51 ` [PATCH 1/1] lib/random.c fix undefined behavior on signed integer overflow Lukas Herbolt
@ 2026-10-07 17:00 ` Darrick J. Wong
2026-10-08 8:23 ` Lukas Herbolt
0 siblings, 1 reply; 5+ messages in thread
From: Darrick J. Wong @ 2026-10-07 17:00 UTC (permalink / raw)
To: Lukas Herbolt; +Cc: zlang, fstests, linux-xfs, linux-ext4
On Tue, Oct 06, 2026 at 03:51:52PM +0200, Lukas Herbolt wrote:
> The random.c now expects that signed integer to overflow and
> triggers the ^MASK branch. But signed int overflow is undefined
> behavior and GCC can optimize this branch out with certain
> CFLAGS/LDFLAGS.
>
> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> ---
> lib/random.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/lib/random.c b/lib/random.c
> index d5c81be817c8..f767ed1d1337 100644
> --- a/lib/random.c
> +++ b/lib/random.c
> @@ -186,14 +186,15 @@ _random (int32_t is [2])
> int32_t
> _irandm (int32_t is [2])
> {
> - int32_t it, leh, nit;
> -
> + int32_t it, leh, nit, apply_mask;
> it = is [0];
> leh = is [1];
> - if (it <= 0)
> - it = (it + it) ^ MASK;
> - else
> - it = it + it;
> +
> +/* evaluate on original value — no UB here */
> + apply_mask = (it <= 0);
> +/* double via unsigned shift — well-defined */
> + it = (unsigned)it << 1;
> + if (apply_mask) it ^= MASK;
Doesn't _random suffer the same flaw?
Oh. It's dead code, maybe it should go away?
Also, what about converting all the variables to unsigned?
--D
> nit = it - 1;
> /* to ensure all-ones pattern omitted */
> leh = leh * mt[nit & 127] + nit;
> --
> 2.55.0
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] lib/random.c fix undefined behavior on signed integer overflow.
2026-10-07 17:00 ` Darrick J. Wong
@ 2026-10-08 8:23 ` Lukas Herbolt
2026-10-08 15:17 ` Darrick J. Wong
0 siblings, 1 reply; 5+ messages in thread
From: Lukas Herbolt @ 2026-10-08 8:23 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: zlang, fstests, linux-xfs, linux-ext4
On 2026-10-07 19:00, Darrick J. Wong wrote:
> On Tue, Oct 06, 2026 at 03:51:52PM +0200, Lukas Herbolt wrote:
>> The random.c now expects that signed integer to overflow and
>> triggers the ^MASK branch. But signed int overflow is undefined
>> behavior and GCC can optimize this branch out with certain
>> CFLAGS/LDFLAGS.
>>
>> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
>> ---
>> lib/random.c | 13 +++++++------
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/lib/random.c b/lib/random.c
>> index d5c81be817c8..f767ed1d1337 100644
>> --- a/lib/random.c
>> +++ b/lib/random.c
>> @@ -186,14 +186,15 @@ _random (int32_t is [2])
>> int32_t
>> _irandm (int32_t is [2])
>> {
>> - int32_t it, leh, nit;
>> -
>> + int32_t it, leh, nit, apply_mask;
>> it = is [0];
>> leh = is [1];
>> - if (it <= 0)
>> - it = (it + it) ^ MASK;
>> - else
>> - it = it + it;
>> +
>> +/* evaluate on original value — no UB here */
>> + apply_mask = (it <= 0);
>> +/* double via unsigned shift — well-defined */
>> + it = (unsigned)it << 1;
>> + if (apply_mask) it ^= MASK;
>
> Doesn't _random suffer the same flaw?
>
> Oh. It's dead code, maybe it should go away?
Good point.
>
> Also, what about converting all the variables to unsigned?
It would break two signed checks: `it <= 0` (LFSR feedback -
only fires at zero instead of when MSB set) and `leh < 0`
(output folding — never fires). The `it` register degenerates
to 33 repeating values after 64 calls. Reducing the pool of
random numbers.
--
-lhe
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] lib/random.c fix undefined behavior on signed integer overflow.
2026-10-08 8:23 ` Lukas Herbolt
@ 2026-10-08 15:17 ` Darrick J. Wong
0 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2026-10-08 15:17 UTC (permalink / raw)
To: Lukas Herbolt; +Cc: zlang, fstests, linux-xfs, linux-ext4
On Thu, Oct 08, 2026 at 10:23:59AM +0200, Lukas Herbolt wrote:
> On 2026-10-07 19:00, Darrick J. Wong wrote:
> > On Tue, Oct 06, 2026 at 03:51:52PM +0200, Lukas Herbolt wrote:
> > > The random.c now expects that signed integer to overflow and
> > > triggers the ^MASK branch. But signed int overflow is undefined
> > > behavior and GCC can optimize this branch out with certain
> > > CFLAGS/LDFLAGS.
> > >
> > > Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> > > ---
> > > lib/random.c | 13 +++++++------
> > > 1 file changed, 7 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/lib/random.c b/lib/random.c
> > > index d5c81be817c8..f767ed1d1337 100644
> > > --- a/lib/random.c
> > > +++ b/lib/random.c
> > > @@ -186,14 +186,15 @@ _random (int32_t is [2])
> > > int32_t
> > > _irandm (int32_t is [2])
> > > {
> > > - int32_t it, leh, nit;
> > > -
> > > + int32_t it, leh, nit, apply_mask;
> > > it = is [0];
> > > leh = is [1];
> > > - if (it <= 0)
> > > - it = (it + it) ^ MASK;
> > > - else
> > > - it = it + it;
> > > +
> > > +/* evaluate on original value — no UB here */
> > > + apply_mask = (it <= 0);
> > > +/* double via unsigned shift — well-defined */
> > > + it = (unsigned)it << 1;
> > > + if (apply_mask) it ^= MASK;
> >
> > Doesn't _random suffer the same flaw?
> >
> > Oh. It's dead code, maybe it should go away?
> Good point.
> >
> > Also, what about converting all the variables to unsigned?
>
> It would break two signed checks: `it <= 0` (LFSR feedback -
> only fires at zero instead of when MSB set) and `leh < 0`
> (output folding — never fires). The `it` register degenerates
> to 33 repeating values after 64 calls. Reducing the pool of
> random numbers.
Ah. Well in that case I'm satisfied, so
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-08 15:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-06 13:51 [PATCH 0/1] Fix random.c UB on int overflow Lukas Herbolt
2026-10-06 13:51 ` [PATCH 1/1] lib/random.c fix undefined behavior on signed integer overflow Lukas Herbolt
2026-10-07 17:00 ` Darrick J. Wong
2026-10-08 8:23 ` Lukas Herbolt
2026-10-08 15:17 ` Darrick J. Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox