[LTP] [PATCH v2] overcommit_memory: Fix integer overflow and 32-bit limits

[Ben Copeland]

Hello Petr,

On Wed, 15 Oct 2025 at 17:02, Petr Vorel <pvorel@suse.cz> wrote:
>
> > On Wed, Oct 15, 2025 at 04:42:47PM +0200, Petr Vorel wrote:
> > > > The overcommit test uses sum_total, the sum (memory and swap) to test
> > > > the overcommit settings.
>
> > > > This fixes two problems on 32-bit systems. The first is seen with a
> > > > integer overflow can occur when calculating sum_total * 2, if the
> > > > sum_total is larger than 2GB. The second is limited virtual address
>
> > > You still mention GB ...
>
>
> > Yep.  It is GB.
>
> > > > space (2-3GB) means the test can fail from address space exhaustion
> > > > before overcommit has been tested.
>
> > > > Now the test runs correctly on low-memory 32-bit systems while avoiding
> > > > both the overflow bug and virtual address space issues.
>
> > > > Signed-off-by: Ben Copeland <ben.copeland@linaro.org>
> > > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > > Reviewed-by: Petr Vorel <pvorel@suse.cz>
> > > > ---
> > > >  .../kernel/mem/tunable/overcommit_memory.c    | 33 +++++++++++++++----
> > > >  1 file changed, 27 insertions(+), 6 deletions(-)
>
> > > > diff --git a/testcases/kernel/mem/tunable/overcommit_memory.c b/testcases/kernel/mem/tunable/overcommit_memory.c
> > > > index 9b2cb479d..7ff5a98d0 100644
> > > > --- a/testcases/kernel/mem/tunable/overcommit_memory.c
> > > > +++ b/testcases/kernel/mem/tunable/overcommit_memory.c
> > > > @@ -131,24 +131,45 @@ static void overcommit_memory_test(void)
> > > >   TST_SYS_CONF_LONG_SET(OVERCOMMIT_MEMORY, 2, 1);
>
> > > >   update_mem_commit();
> > > > - alloc_and_check(commit_left * 2, EXPECT_FAIL);
> > > > - alloc_and_check(commit_limit + total_batch_size, EXPECT_FAIL);
> > > > + /* Skip tests that would overflow or exceed 32-bit address space */
> > > > + if (tst_kernel_bits() == 64 || (unsigned long)commit_left <= TST_GB / TST_KB) {
>
> > > ... but TST_GB / TST_KB is actually MB (you could use TST_MB).
>
>
> > The sizes in this test are measured in KB, so it's 1GB but measured in
> > terms of KB not bytes.  Using TST_MB would work mathematically but it's
> > misleading.
>
> Ah, I'm sorry to overlook an obvious point.
>
> Implementation details: thinking about the code twice, shouldn't be the check
> for overflow in alloc_and_check() instead of outside (to keep the condition on
> single place)?

Excellent suggestion. It makes sense to simplify it like that. I will
spin up a v3 patch with those changes.


Ben



>
> Also, if kept outside the 1st and 2nd if could be joined:
>
>         /* Skip tests that would overflow or exceed 32-bit address space */
>         if (tst_kernel_bits() == 64 || (unsigned long)commit_left <= TST_GB / TST_KB) {
>                 alloc_and_check(commit_left * 2, EXPECT_FAIL);
>                 alloc_and_check(commit_limit + total_batch_size, EXPECT_FAIL);
>                 update_mem_commit();
>                 alloc_and_check(commit_left / 2, EXPECT_PASS);
>         } else {
>                 tst_res(TCONF, "Skipping large allocation tests due to address space limits");
>         }
>
> instead of your proposal:
>
>         update_mem_commit();
>         /* Skip tests that would overflow or exceed 32-bit address space */
>         if (tst_kernel_bits() == 64 || (unsigned long)commit_left <= TST_GB / TST_KB) {
>                 alloc_and_check(commit_left * 2, EXPECT_FAIL);
>                 alloc_and_check(commit_limit + total_batch_size, EXPECT_FAIL);
>         } else {
>                 tst_res(TCONF, "Skipping large allocation tests due to address space limits");
>         }
>         update_mem_commit();
>         if (tst_kernel_bits() == 64 || (unsigned long)commit_left <= TST_GB / TST_KB) {
>                 alloc_and_check(commit_left / 2, EXPECT_PASS);
>         } else {
>                 tst_res(TCONF, "Skipping commit_left/2 allocation test due to address space limits");
>         }
>
> because update_mem_commit() IMHO just evaluates /proc/meminfo values, but
> when alloc_and_check() is skipped nothing has changed.
>
> Kind regards,
> Petr
>
> > regards,
> > dan carpenter
>