[PATCH v2] selftests/mm: mlock2-tests: fix fclose(NULL) in is_vmflag_set()

[PATCH v2] selftests/mm: mlock2-tests: fix fclose(NULL) in is_vmflag_set()

[Hongfu Li]

is_vmflag_set() jumps to the shared 'out' label when
seek_to_smaps_entry() fails. smaps is NULL on that path, and the
code at the 'out' label calls fclose(smaps) unconditionally.
fclose(NULL) is undefined behaviour, so the test crashes before
it can report a test result.

Return early on that error path instead, so smaps is always valid
at the 'out' label.

get_value_for_name() follows the same pattern. Return early on the
seek_to_smaps_entry() error path there as well, and drop the now
redundant 'if (smaps)' guard on the fclose().

Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
---
Changes in v2:
- Convert get_value_for_name() in the same way, as suggested by Liam R.Howlett.
- Update the commit message.
---
 tools/testing/selftests/mm/mlock2-tests.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/mm/mlock2-tests.c b/tools/testing/selftests/mm/mlock2-tests.c
index 144b550813a6..455f1c841dec 100644
--- a/tools/testing/selftests/mm/mlock2-tests.c
+++ b/tools/testing/selftests/mm/mlock2-tests.c
@@ -66,7 +66,7 @@ static bool is_vmflag_set(unsigned long addr, const char *vmflag)
 	smaps = seek_to_smaps_entry(addr);
 	if (!smaps) {
 		ksft_print_msg("Unable to parse /proc/self/smaps\n");
-		goto out;
+		return false;
 	}
 
 	while (getline(&line, &size, smaps) > 0) {
@@ -103,7 +103,7 @@ static unsigned long get_value_for_name(unsigned long addr, const char *name)
 	smaps = seek_to_smaps_entry(addr);
 	if (!smaps) {
 		ksft_print_msg("Unable to parse /proc/self/smaps\n");
-		goto out;
+		return value;
 	}
 
 	while (getline(&line, &size, smaps) > 0) {
@@ -123,9 +123,8 @@ static unsigned long get_value_for_name(unsigned long addr, const char *name)
 	}
 
 out:
-	if (smaps)
-		fclose(smaps);
 	free(line);
+	fclose(smaps);
 	return value;
 }
 

---
base-commit: 74d73ffcffe03804f97e9fb82301dadf75671282
change-id: 20261104-for-mlock2-a67f86dad498

Best regards,
--  
Hongfu Li <lihongfu@kylinos.cn>

Re: [PATCH v2] selftests/mm: mlock2-tests: fix fclose(NULL) in is_vmflag_set()

[David Hildenbrand (Arm)]

On 11/13/26 03:52, Hongfu Li wrote:
> is_vmflag_set() jumps to the shared 'out' label when
> seek_to_smaps_entry() fails. smaps is NULL on that path, and the
> code at the 'out' label calls fclose(smaps) unconditionally.
> fclose(NULL) is undefined behaviour, so the test crashes before
> it can report a test result.

I was briefly confuses staring at get_value_for_name() diff below. WHich is
clarified below.

> 
> Return early on that error path instead, so smaps is always valid
> at the 'out' label.
> 
> get_value_for_name() follows the same pattern. Return early on the
> seek_to_smaps_entry() error path there as well, and drop the now
> redundant 'if (smaps)' guard on the fclose().
> 
> Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>

Fixes: tag? :)

Apart form that LGTM

Reviewed-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH v2] selftests/mm: mlock2-tests: fix fclose(NULL) in is_vmflag_set()

[Lorenzo Stoakes (ARM)]

On Fri, Nov 13, 2026 at 11:31:55AM +0100, David Hildenbrand (Arm) wrote:
> On 11/13/26 03:52, Hongfu Li wrote:
> > is_vmflag_set() jumps to the shared 'out' label when
> > seek_to_smaps_entry() fails. smaps is NULL on that path, and the
> > code at the 'out' label calls fclose(smaps) unconditionally.
> > fclose(NULL) is undefined behaviour, so the test crashes before
> > it can report a test result.
>
> I was briefly confuses staring at get_value_for_name() diff below. WHich is
> clarified below.
>
> >
> > Return early on that error path instead, so smaps is always valid
> > at the 'out' label.
> >
> > get_value_for_name() follows the same pattern. Return early on the
> > seek_to_smaps_entry() error path there as well, and drop the now
> > redundant 'if (smaps)' guard on the fclose().
> >
> > Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
>
> Fixes: tag? :)
>
> Apart form that LGTM
>
> Reviewed-by: David Hildenbrand (Arm) <david@kernel.org>

Similarly,

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo