linux-kernel - Re: [PATCH v2 3/4] selftests/mm: Report unique test names for each cow test

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: <9961082f-848d-43d3-b97d-3df675ca4415@sirena.org.uk>
Date: Tue, 3 Jun 2025 14:21:21 +0100
From: Mark Brown <broonie@...nel.org>
To: David Hildenbrand <david@...hat.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	Shuah Khan <shuah@...nel.org>,
	Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, linux-mm@...ck.org,
	linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/4] selftests/mm: Report unique test names for each
 cow test

On Tue, Jun 03, 2025 at 02:51:45PM +0200, David Hildenbrand wrote:
> On 27.05.25 18:04, Mark Brown wrote:

> >   		ret = mprotect(mem, size, PROT_READ);
> > -		ret |= mprotect(mem, size, PROT_READ|PROT_WRITE);
> >   		if (ret) {

> Not sure if that change is really required: if the second mprotect succeeds,
> errno should not be updated. At least if my memory is correct :)

> Same applies to similar cases below.

I thought about checking to see if that was guaranteed to be the case,
then I thought that if that wasn't clear to me right now without
checking it probably also wasn't going to be obvious to future readers
so it was better to just write something clear.  Previously we didn't
report errno so it didn't matter.

> >   	} else {
> > -		ksft_test_result_fail("Leak from parent into child\n");

> Same here and in other cases below (I probably didn't catch all).

> We should log that somehow to indicate what exactly is going wrong, likely
> using ksft_print_msg().

Can you send a patch with the logging that you think would be clear
please?  I dropped these because they just seemed to be reporting the
overall point of the test, unlike the cases where we ran into some error
during the setup and didn't actually manage to perform the test we were
trying to do.  Perhaps the tests should be renamed.

> >   	tmp = malloc(size);
> >   	if (!tmp) {
> > -		ksft_test_result_fail("malloc() failed\n");
> > +		ksft_print_msg("malloc() failed\n");

> perror?

malloc() can only set one errno.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)