lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <091c6181643ca4ce7c124fd2380934be6707cbbb.camel@linux.ibm.com>
Date: Tue, 19 Aug 2025 07:18:55 +0530
From: Aboorva Devarajan <aboorvad@...ux.ibm.com>
To: Wei Yang <richard.weiyang@...il.com>
Cc: akpm@...ux-foundation.org, Liam.Howlett@...cle.com,
        lorenzo.stoakes@...cle.com, shuah@...nel.org, pfalcato@...e.de,
        david@...hat.com, ziy@...dia.com, baolin.wang@...ux.alibaba.com,
        npache@...hat.com, ryan.roberts@....com, dev.jain@....com,
        baohua@...nel.org, linux-mm@...ck.org, linux-kselftest@...r.kernel.org,
        linux-kernel@...r.kernel.org, donettom@...ux.ibm.com,
        ritesh.list@...il.com
Subject: Re: [PATCH v4 5/7] selftests/mm: fix child process exit codes in
 ksm_functional_tests

On Sat, 2025-08-16 at 14:43 +0000, Wei Yang wrote:

Hi Wei,

> On Sat, Aug 16, 2025 at 09:31:11AM +0530, Aboorva Devarajan wrote:
> > In ksm_functional_tests, test_child_ksm() returned negative values to
> > indicate errors.  However, when passed to exit(), these were interpreted
> > as large unsigned values (e.g, -2 became 254), leading to incorrect
> > handling in the parent process.  As a result, some tests appeared to be
> > skipped or silently failed.
> 
> This is because "the least significant 8 bits" is returned to parent, right?
> 
> 

Yes, that's right. As per the WEXITSTATUS(wstatus) manual:

WEXITSTATUS: returns the exit status of the child. This consists of the
least significant 8 bits of the status argument that the child
specified in a call to exit(3) or _exit(2) or as the argument for a
return statement in main(). This macro should only be employed if
WIFEXITED returned true.

Since only the least significant 8 bits are preserved, negative return
values can appear as large unsigned codes, so using small positive exit
codes ensures the parent interprets the error code correctly.


> > This patch changes test_child_ksm() to return positive error codes (1, 2,
> > 3) and updates test_child_ksm_err() to interpret them correctly.
> > Additionally, test_prctl_fork_exec() now uses exit(4) after a failed
> > execv() to clearly signal exec failures.  This ensures the parent
> > accurately detects and reports child process failures.
> > 
> > --------------
> > Before patch:
> > --------------
> > - [RUN] test_unmerge
> > ok 1 Pages were unmerged
> > ...
> > - [RUN] test_prctl_fork
> > - No pages got merged
> > - [RUN] test_prctl_fork_exec
> > ok 7 PR_SET_MEMORY_MERGE value is inherited
> > ...
> > Bail out! 1 out of 8 tests failed
> > - Planned tests != run tests (9 != 8)
> > - Totals: pass:7 fail:1 xfail:0 xpass:0 skip:0 error:0
> > 
> > --------------
> > After patch:
> > --------------
> > - [RUN] test_unmerge
> > ok 1 Pages were unmerged
> > ...
> > - [RUN] test_prctl_fork
> > - No pages got merged
> > not ok 7 Merge in child failed
> > - [RUN] test_prctl_fork_exec
> > ok 8 PR_SET_MEMORY_MERGE value is inherited
> > ...
> > Bail out! 2 out of 9 tests failed
> > - Totals: pass:7 fail:2 xfail:0 xpass:0 skip:0 error:0
> > 
> > Fixes: 6c47de3be3a0 ("selftest/mm: ksm_functional_tests: extend test case for ksm fork/exec")
> > Co-developed-by: Donet Tom <donettom@...ux.ibm.com>
> > Signed-off-by: Donet Tom <donettom@...ux.ibm.com>
> > Acked-by: David Hildenbrand <david@...hat.com>
> > Signed-off-by: Aboorva Devarajan <aboorvad@...ux.ibm.com>
> 
> If so:
> 
> Reviewed-by: Wei Yang <richard.weiyang@...il.com>
> 
> Thanks, I am afraid to make the same mistake if you don't point out.

Thanks,
Aboorva
	

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ