linux-kernel - Re: [RFC v2 PATCH 3/3] selftests/devmem: initial testset

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: <2025102151-distill-operate-a748@gregkh>
Date: Tue, 21 Oct 2025 09:35:05 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Gabriele Paoloni <gpaoloni@...hat.com>
Cc: shuah@...nel.org, linux-kselftest@...r.kernel.org,
	linux-kernel@...r.kernel.org, corbet@....net,
	linux-doc@...r.kernel.org, linux-mm@...ck.org,
	safety-architecture@...ts.elisa.tech, acarmina@...hat.com,
	kstewart@...uxfoundation.org, chuckwolber@...il.com
Subject: Re: [RFC v2 PATCH 3/3] selftests/devmem: initial testset

On Wed, Sep 10, 2025 at 07:00:00PM +0200, Gabriele Paoloni wrote:
> From: Alessandro Carminati <acarmina@...hat.com>
> 
> This patch introduces a new series of tests for devmem.
> Test cases are mapped against the tested Function's expectations
> defined in /drivers/char/mem.c.

Cool, but:

> 
> Signed-off-by: Alessandro Carminati <acarmina@...hat.com>
> ---
>  tools/testing/selftests/Makefile         |   1 +
>  tools/testing/selftests/devmem/Makefile  |  13 +
>  tools/testing/selftests/devmem/debug.c   |  25 +
>  tools/testing/selftests/devmem/debug.h   |  14 +
>  tools/testing/selftests/devmem/devmem.c  | 200 ++++++++
>  tools/testing/selftests/devmem/ram_map.c | 250 ++++++++++
>  tools/testing/selftests/devmem/ram_map.h |  38 ++
>  tools/testing/selftests/devmem/secret.c  |  46 ++
>  tools/testing/selftests/devmem/secret.h  |  13 +
>  tools/testing/selftests/devmem/tests.c   | 569 +++++++++++++++++++++++
>  tools/testing/selftests/devmem/tests.h   |  45 ++
>  tools/testing/selftests/devmem/utils.c   | 379 +++++++++++++++
>  tools/testing/selftests/devmem/utils.h   | 119 +++++

That's a lot of files for a "simple" test.  Doesn't LTP have tests for
this api already?  Why not use that here instead?

Also, this is userspace testing, not kunit testing, right, is that
intentional?  You are documenting internal apis and then writing
userspace tests for those apis, which feels a bit odd.

Also /dev/mem should not be used on "modern" systems, so how was this
tested?

> +// SPDX-License-Identifier: GPL-2.0+

Are you _sure_ you want GPLv2+?  I have to ask, sorry.

> +/*
> + * devmem test debug.c
> + *
> + * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved.
> + * Written by Alessandro Carminati (acarmina@...hat.com)
> + */
> +
> +#include <stdio.h>
> +#include <stdarg.h>
> +
> +#define DEBUG_FLAG 0
> +int pdebug = DEBUG_FLAG;

That's a funny define that is never used elsewhere.  I'm guessing this
was cut/pasted from some other userspace code somewhere?

> +
> +void deb_printf(const char *fmt, ...)

Who is "deb"?  You have more letters, always use them :)

Also, why debugging for just this one set of tests?  Don't kselftests
already have debugging logic?  if not, why is this unique to require it?

And am I missing something, or does this new tool not tie into the
kselftest framework properly?  I see lots of printing to output, but not
in the proper test framework format, am I just missing that somewhere?

thanks,

greg k-h