linux-kernel - Re: [PATCH 3/3] selftests: ublk: add test to verify that feat

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] [day] [month] [year] [list]

Message-ID: <CAFj5m9L3+=UsMzM3JU3QXw3tVqYjZs7s8QHbFy_KaG4Rs_z_VQ@mail.gmail.com>
Date: Thu, 18 Sep 2025 08:23:36 +0800
From: Ming Lei <ming.lei@...hat.com>
To: Uday Shankar <ushankar@...estorage.com>
Cc: Caleb Sander Mateos <csander@...estorage.com>, Shuah Khan <shuah@...nel.org>, 
	linux-block@...r.kernel.org, linux-kselftest@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] selftests: ublk: add test to verify that feat_map is complete

On Thu, Sep 18, 2025 at 3:13 AM Uday Shankar <ushankar@...estorage.com> wrote:
>
> On Wed, Sep 17, 2025 at 11:52:38AM +0800, Ming Lei wrote:
> > On Tue, Sep 16, 2025 at 04:05:57PM -0600, Uday Shankar wrote:
> > > Add a test that verifies that the currently running kernel does not
> > > report support for any features that are unrecognized by kublk. This
> > > should catch cases where features are added without updating kublk's
> > > feat_map accordingly, which has happened multiple times in the past (see
> > > [1], [2]).
> > >
> > > Note that this new test may fail if the test suite is older than the
> > > kernel, and the newer kernel contains a newly introduced feature. I
> > > believe this is not a use case we currently care about - we only care
> > > about newer test suites passing on older kernels.
> > >
> > > [1] https://lore.kernel.org/linux-block/20250606214011.2576398-1-csander@purestorage.com/t/#u
> > > [2] https://lore.kernel.org/linux-block/2a370ab1-d85b-409d-b762-f9f3f6bdf705@nvidia.com/t/#m1c520a058448d594fd877f07804e69b28908533f
> > >
> > > Signed-off-by: Uday Shankar <ushankar@...estorage.com>
> > > ---
> > >  tools/testing/selftests/ublk/Makefile           |  1 +
> > >  tools/testing/selftests/ublk/test_generic_13.sh | 16 ++++++++++++++++
> > >  2 files changed, 17 insertions(+)
> > >
> > > diff --git a/tools/testing/selftests/ublk/Makefile b/tools/testing/selftests/ublk/Makefile
> > > index 5d7f4ecfb81612f919a89eb442f948d6bfafe225..770269efe42ab460366485ccc80abfa145a0c57b 100644
> > > --- a/tools/testing/selftests/ublk/Makefile
> > > +++ b/tools/testing/selftests/ublk/Makefile
> > > @@ -20,6 +20,7 @@ TEST_PROGS += test_generic_09.sh
> > >  TEST_PROGS += test_generic_10.sh
> > >  TEST_PROGS += test_generic_11.sh
> > >  TEST_PROGS += test_generic_12.sh
> > > +TEST_PROGS += test_generic_13.sh
> > >
> > >  TEST_PROGS += test_null_01.sh
> > >  TEST_PROGS += test_null_02.sh
> > > diff --git a/tools/testing/selftests/ublk/test_generic_13.sh b/tools/testing/selftests/ublk/test_generic_13.sh
> > > new file mode 100755
> > > index 0000000000000000000000000000000000000000..ff5f22b078ddd08bc19f82aa66da6a44fa073f6f
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/ublk/test_generic_13.sh
> > > @@ -0,0 +1,16 @@
> > > +#!/bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +
> > > +. "$(cd "$(dirname "$0")" && pwd)"/test_common.sh
> > > +
> > > +TID="generic_13"
> > > +ERR_CODE=0
> > > +
> > > +_prep_test "null" "check that feature list is complete"
> > > +
> > > +if ${UBLK_PROG} features | grep -q unknown; then
> > > +        ERR_CODE=255
> > > +fi
> > > +
> > > +_cleanup_test "null"
> > > +_show_result $TID $ERR_CODE
> >
> > What if the ublk selftest is run on downstream kernel?
> >
> > Maybe the output can changed to "unsupported" to show that ublk selftest
> > code doesn't cover or use this feature.
>
> Yes I pointed out this issue in the commit message too. But I am unsure
> what you're asking for.
>
> I think we need a failure here if this test is to fulfill its intended
> purposes (catching cases where a feature is added without updating the
> feat_map in kublk). This does also cause failures in cases where an old
> test suite is run against a newer kernel. Since I think this is an
> unusual case, perhaps I can add a log line when this test fails saying

That also requires any new feature to have a selftest to cover, but sounds good,
and should be the motivation of this patch.

> that the failure is expected if running an old test suite against a new
> kernel?

Fair enough, then this patch looks fine for me:

Reviewed-by: Ming Lei <ming.lei@...hat.com>

Thanks,