- Linux-kselftest-mirror - lists.linaro.org

[PATCH 0/4] Fix kselftest build with sub-directory

by Guillaume Tucker

Earlier attempts to get "make O=build kselftest-all" to work were not successful as they made undesirable changes to some functions in the top-level Makefile. This series takes a different approach by removing the root cause of the problem within kselftest, which is when the sub-Makefile tries to install kernel headers "backwards" by calling make with the top-level Makefile. The actual issue comes from the fact that $(srctree) is ".." when building in a sub-directory with "O=build" which then obviously makes "-C $(top_srcdir)" point outside of the real source tree. With this series, the generic kselftest targets work as expected from the top level with or without a build directory e.g.: $ make kselftest-all $ make O=build kselftest-all Then in order to build using the sub-Makefile explicitly, the headers have to be installed first. This is arguably a valid requirement to have when building a tool from a sub-Makefile. For example, "make -C tools/testing/nvdimm/" fails in a similar way until <asm/rwonce.h> has been generated by a kernel build. Guillaume Tucker (4): selftests: drop khdr make target selftests: stop using KSFT_KHDR_INSTALL selftests: drop KSFT_KHDR_INSTALL make target Makefile: add headers_install to kselftest targets Makefile | 4 +- tools/testing/selftests/Makefile | 28 +------------- tools/testing/selftests/arm64/mte/Makefile | 1 - tools/testing/selftests/arm64/signal/Makefile | 1 - .../selftests/arm64/signal/test_signals.h | 4 +- .../selftests/drivers/s390x/uvdevice/Makefile | 1 - .../selftests/futex/functional/Makefile | 1 - tools/testing/selftests/kvm/Makefile | 1 - tools/testing/selftests/landlock/Makefile | 1 - tools/testing/selftests/lib.mk | 38 ------------------- tools/testing/selftests/net/Makefile | 1 - tools/testing/selftests/net/mptcp/Makefile | 1 - tools/testing/selftests/tc-testing/Makefile | 1 - tools/testing/selftests/vm/Makefile | 1 - 14 files changed, 5 insertions(+), 79 deletions(-) -- 2.30.2

3 years

5
11
0 0

[PATCH v7 0/7] bpf: Add bpf_verify_pkcs7_signature() helper

by Roberto Sassu

One of the desirable features in security is the ability to restrict import of data to a given system based on data authenticity. If data import can be restricted, it would be possible to enforce a system-wide policy based on the signing keys the system owner trusts. This feature is widely used in the kernel. For example, if the restriction is enabled, kernel modules can be plugged in only if they are signed with a key whose public part is in the primary or secondary keyring. For eBPF, it can be useful as well. For example, it might be useful to authenticate data an eBPF program makes security decisions on. After a discussion in the eBPF mailing list, it was decided that the stated goal should be accomplished by introducing a new helper: bpf_verify_pkcs7_signature(), dedicated to verify PKCS#7 signatures. Other than the data and the signature, the helper also receives two parameters for the keyring, which can be provided as alternatives: one is a key pointer returned by the new bpf_lookup_user_key() helper, called with a key serial possibly decided by the user; another is a pre-determined ID among values defined in include/linux/verification.h. While the first keyring-related parameter provides great flexibility, it seems suboptimal in terms of security guarantees, as even if the eBPF program is assumed to be trusted, the serial used to obtain the key pointer might come from untrusted user space not choosing one that the system administrator approves to enforce a mandatory policy. The second keyring-related parameter instead provides much stronger guarantees, especially if the pre-determined ID is not passed by user space but is hardcoded in the eBPF program, and that program is signed. In this case, bpf_verify_pkcs7_signature() will always perform signature verification with a key that the system administrator approves, i.e. the primary, secondary or platform keyring. bpf_lookup_user_key() comes with the corresponding release helper bpf_key_put(), to decrement the reference count of the key found with the former helper. The eBPF verifier has been enhanced to ensure that the release helper is always called whenever the acquire helper is called, or otherwise refuses to load the program. bpf_lookup_user_key() also accepts lookup-specific flags KEY_LOOKUP_CREATE and KEY_LOOKUP_PARTIAL. Although these are most likely not useful for the bpf_verify_pkcs7_signature(), newly defined flags could be. bpf_lookup_user_key() does not request a particular permission to lookup_user_key(), as it cannot determine it by itself. Also, it should not get it from the user, as the user could pass an arbitrary value and use the key for a different purpose. Instead, bpf_lookup_user_key() requests KEY_DEFER_PERM_CHECK, and defers the permission check to the helper that actually uses the key, in this patch set to bpf_verify_pkcs7_signature(). Since key_task_permission() is called by the PKCS#7 code during signature verification, the only additional function bpf_verify_pkcs7_signature() has to call is key_validate(). With that, the permission check can be considered complete and equivalent, as it was done by bpf_lookup_user_key() with the appropriate permission (in this case KEY_NEED_SEARCH). All helpers can be called only from sleepable programs, because of memory allocation (with lookup flag KEY_LOOKUP_CREATE) and crypto operations. For example, the lsm.s/bpf attach point is suitable, fexit/array_map_update_elem is not. The correctness of implementation of the new helpers and of their usage is checked with the introduced tests. The patch set is organized as follows. Patch 1 exports bpf_dynptr_get_size(), to obtain the real size of data carried by a dynamic pointer. Patch 2 makes available for new eBPF helpers some key-related definitions. Patch 3 introduces the bpf_lookup_user_key() and bpf_key_put() helpers. Patch 4 introduces the bpf_verify_pkcs7_signature(). Finally, patches 5-7 introduce the tests. Changelog v6: - Switch back to key lookup helpers + signature verification (until v5), and defer permission check from bpf_lookup_user_key() to bpf_verify_pkcs7_signature() - Add additional key lookup test to illustrate the usage of the KEY_LOOKUP_CREATE flag and validate the flags (suggested by Daniel) - Make description of flags of bpf_lookup_user_key() more user-friendly (suggested by Daniel) - Fix validation of flags parameter in bpf_lookup_user_key() (reported by Daniel) - Rename bpf_verify_pkcs7_signature() keyring-related parameters to user_keyring and system_keyring to make their purpose more clear - Accept keyring-related parameters of bpf_verify_pkcs7_signature() as alternatives (suggested by KP) - Replace unsigned long type with u64 in helper declaration (suggested by Daniel) - Extend the bpf_verify_pkcs7_signature() test by calling the helper without data, by ensuring that the helper enforces the keyring-related parameters as alternatives, by ensuring that the helper rejects inaccessible and expired keyrings, and by checking all system keyrings - Move bpf_lookup_user_key() and bpf_key_put() usage tests to ref_tracking.c (suggested by John) - Call bpf_lookup_user_key() and bpf_key_put() only in sleepable programs v5: - Move KEY_LOOKUP_ to include/linux/key.h for validation of bpf_verify_pkcs7_signature() parameter - Remove bpf_lookup_user_key() and bpf_key_put() helpers, and the corresponding tests - Replace struct key parameter of bpf_verify_pkcs7_signature() with the keyring serial and lookup flags - Call lookup_user_key() and key_put() in bpf_verify_pkcs7_signature() code, to ensure that the retrieved key is used according to the permission requested at lookup time - Clarified keyring precedence in the description of bpf_verify_pkcs7_signature() (suggested by John) - Remove newline in the second argument of ASSERT_ - Fix helper prototype regular expression in bpf_doc.py v4: - Remove bpf_request_key_by_id(), don't return an invalid pointer that other helpers can use - Pass the keyring ID (without ULONG_MAX, suggested by Alexei) to bpf_verify_pkcs7_signature() - Introduce bpf_lookup_user_key() and bpf_key_put() helpers (suggested by Alexei) - Add lookup_key_norelease test, to ensure that the verifier blocks eBPF programs which don't decrement the key reference count - Parse raw PKCS#7 signature instead of module-style signature in the verify_pkcs7_signature test (suggested by Alexei) - Parse kernel module in user space and pass raw PKCS#7 signature to the eBPF program for signature verification v3: - Rename bpf_verify_signature() back to bpf_verify_pkcs7_signature() to avoid managing different parameters for each signature verification function in one helper (suggested by Daniel) - Use dynamic pointers and export bpf_dynptr_get_size() (suggested by Alexei) - Introduce bpf_request_key_by_id() to give more flexibility to the caller of bpf_verify_pkcs7_signature() to retrieve the appropriate keyring (suggested by Alexei) - Fix test by reordering the gcc command line, always compile sign-file - Improve helper support check mechanism in the test v2: - Rename bpf_verify_pkcs7_signature() to a more generic bpf_verify_signature() and pass the signature type (suggested by KP) - Move the helper and prototype declaration under #ifdef so that user space can probe for support for the helper (suggested by Daniel) - Describe better the keyring types (suggested by Daniel) - Include linux/bpf.h instead of vmlinux.h to avoid implicit or redeclaration - Make the test selfcontained (suggested by Alexei) v1: - Don't define new map flag but introduce simple wrapper of verify_pkcs7_signature() (suggested by Alexei and KP) Roberto Sassu (7): bpf: Export bpf_dynptr_get_size() KEYS: Move KEY_LOOKUP_ to include/linux/key.h bpf: Add bpf_lookup_user_key() and bpf_key_put() helpers bpf: Add bpf_verify_pkcs7_signature() helper selftests: Add verifier tests for bpf_lookup_user_key() and bpf_key_put() selftests/bpf: Add additional test for bpf_lookup_user_key() selftests/bpf: Add test for bpf_verify_pkcs7_signature() helper include/linux/bpf.h | 1 + include/linux/key.h | 3 + include/uapi/linux/bpf.h | 47 ++ kernel/bpf/bpf_lsm.c | 116 +++++ kernel/bpf/helpers.c | 2 +- kernel/bpf/verifier.c | 6 +- scripts/bpf_doc.py | 2 + security/keys/internal.h | 2 - tools/include/uapi/linux/bpf.h | 47 ++ tools/testing/selftests/bpf/Makefile | 14 +- tools/testing/selftests/bpf/config | 2 + .../bpf/prog_tests/lookup_user_key.c | 94 ++++ .../bpf/prog_tests/verify_pkcs7_sig.c | 410 ++++++++++++++++++ .../bpf/progs/test_lookup_user_key.c | 35 ++ .../bpf/progs/test_verify_pkcs7_sig.c | 90 ++++ tools/testing/selftests/bpf/test_verifier.c | 3 +- .../selftests/bpf/verifier/ref_tracking.c | 66 +++ .../testing/selftests/bpf/verify_sig_setup.sh | 104 +++++ 18 files changed, 1035 insertions(+), 9 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/lookup_user_key.c create mode 100644 tools/testing/selftests/bpf/prog_tests/verify_pkcs7_sig.c create mode 100644 tools/testing/selftests/bpf/progs/test_lookup_user_key.c create mode 100644 tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c create mode 100755 tools/testing/selftests/bpf/verify_sig_setup.sh -- 2.25.1

3 years

2
8
0 0

[PATCH v2] kunit: executor: Fix a memory leak on failure in kunit_filter_tests

by David Gow

It's possible that memory allocation for 'filtered' will fail, but for the copy of the suite to succeed. In this case, the copy could be leaked. Properly free 'copy' in the error case for the allocation of 'filtered' failing. Note that there may also have been a similar issue in kunit_filter_subsuites, before it was removed in "kunit: flatten kunit_suite*** to kunit_suite** in .kunit_test_suites". This was reported by clang-analyzer via the kernel test robot, here: https://lore.kernel.org/all/c8073b8e-7b9e-0830-4177-87c12f16349c@intel.com/ And by smatch via Dan Carpenter and the kernel test robot: https://lore.kernel.org/all/202207101328.ASjx88yj-lkp@intel.com/ Fixes: a02353f49162 ("kunit: bail out of test filtering logic quicker if OOM") Reported-by: kernel test robot <yujie.liu(a)intel.com> Reported-by: kernel test robot <lkp(a)intel.com> Reported-by: Dan Carpenter <dan.carpenter(a)oracle.com> Reviewed-by: Daniel Latypov <dlatypov(a)google.com> Reviewed-by: Brendan Higgins <brendanhiggins(a)google.com> Signed-off-by: David Gow <davidgow(a)google.com> --- Thanks everyone! No actual code changes in v2, just fixes to the description. Changes since v1: https://lore.kernel.org/all/20220712095627.1770312-1-davidgow@google.com/ - Fix a mistake in the commit description where we noted the allocation for 'copy' could fail, instead of 'filtered'. (Thanks Yujie!) - Noted in the description that smatch also found this (Thanks Dan!) - Added the extra Reported-by and Reviewed-by tags. --- lib/kunit/executor.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 6c489d6c5e5d..5e223327196a 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -74,8 +74,10 @@ kunit_filter_tests(const struct kunit_suite *const suite, const char *test_glob) return ERR_PTR(-ENOMEM); filtered = kcalloc(n + 1, sizeof(*filtered), GFP_KERNEL); - if (!filtered) + if (!filtered) { + kfree(copy); return ERR_PTR(-ENOMEM); + } n = 0; kunit_suite_for_each_test_case(suite, test_case) { -- 2.37.0.144.g8ac04bfd2-goog

3 years

1
0
0 0

[PATCH] kunit: executor: Fix a memory leak on failure in kunit_filter_tests

by David Gow

It's possible that memory allocation for the copy will fail, but for the copy of the suite to succeed. In this case, the copy could be leaked. Properly free 'copy' in the error case for the allocation of 'filtered' failing. Note that there may also have been a similar issue in kunit_filter_subsuites, before it was removed in "kunit: flatten kunit_suite*** to kunit_suite** in .kunit_test_suites". This was reported by clang-analyzer via the kernel test robot, here: https://lore.kernel.org/all/c8073b8e-7b9e-0830-4177-87c12f16349c@intel.com/ Fixes: a02353f49162 ("kunit: bail out of test filtering logic quicker if OOM") Reported-by: kernel test robot <yujie.liu(a)intel.com> Signed-off-by: David Gow <davidgow(a)google.com> --- lib/kunit/executor.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 6c489d6c5e5d..5e223327196a 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -74,8 +74,10 @@ kunit_filter_tests(const struct kunit_suite *const suite, const char *test_glob) return ERR_PTR(-ENOMEM); filtered = kcalloc(n + 1, sizeof(*filtered), GFP_KERNEL); - if (!filtered) + if (!filtered) { + kfree(copy); return ERR_PTR(-ENOMEM); + } n = 0; kunit_suite_for_each_test_case(suite, test_case) { -- 2.37.0.144.g8ac04bfd2-goog

3 years

4
3
0 0

kselftest/next build: 5 builds: 0 failed, 5 passed (v5.19-rc4-13-g67bd292cd281)

by kernelci.org bot

kselftest/next build: 5 builds: 0 failed, 5 passed (v5.19-rc4-13-g67bd292cd281) Full Build Summary: https://kernelci.org/build/kselftest/branch/next/kernel/v5.19-rc4-13-g67bd2… Tree: kselftest Branch: next Git Describe: v5.19-rc4-13-g67bd292cd281 Git Commit: 67bd292cd281be2216cd269c161be31cd3ccf196 Git URL: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git Built: 2 unique architectures ================================================================================ Detailed per-defconfig build reports: -------------------------------------------------------------------------------- defconfig+kselftest (arm64, gcc-10) — PASS, 0 errors, 0 warnings, 0 section mismatches -------------------------------------------------------------------------------- defconfig+kselftest (arm64, clang-14) — PASS, 0 errors, 0 warnings, 0 section mismatches -------------------------------------------------------------------------------- defconfig+kselftest+arm64-chromebook (arm64, gcc-10) — PASS, 0 errors, 0 warnings, 0 section mismatches -------------------------------------------------------------------------------- defconfig+kselftest+arm64-chromebook (arm64, clang-14) — PASS, 0 errors, 0 warnings, 0 section mismatches -------------------------------------------------------------------------------- x86_64_defconfig+kselftest (x86_64, clang-14) — PASS, 0 errors, 0 warnings, 0 section mismatches --- For more info write to <info(a)kernelci.org>

3 years

1
0
0 0

[PATCH net] selftest: net: add tun to .gitignore

by Jakub Kicinski

Add missing .gitignore entry. Fixes: 839b92fede7b ("selftest: tun: add test for NAPI dismantle") Signed-off-by: Jakub Kicinski <kuba(a)kernel.org> --- tools/testing/selftests/net/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore index a29f79618934..ffc35a22e914 100644 --- a/tools/testing/selftests/net/.gitignore +++ b/tools/testing/selftests/net/.gitignore @@ -36,4 +36,5 @@ test_unix_oob gro ioam6_parser toeplitz +tun cmsg_sender -- 2.36.1

3 years

2
1
0 0

[PATCH] selftests/ftrace: func_event_triggers: fix typo in user message

by Randy Dunlap

Correct typo of "it's" to "it". Signed-off-by: Randy Dunlap <rdunlap(a)infradead.org> Cc: Steven Rostedt <rostedt(a)goodmis.org> Cc: Ingo Molnar <mingo(a)redhat.com> Cc: Shuah Khan <shuah(a)kernel.org> Cc: linux-kselftest(a)vger.kernel.org --- tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- linux-next-20211224.orig/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc +++ linux-next-20211224/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc @@ -85,7 +85,7 @@ run_enable_disable() { echo $check_disable > $EVENT_ENABLE done sleep $SLEEP_TIME - echo " make sure it's still works" + echo " make sure it still works" test_event_enabled $check_enable_star reset_ftrace_filter

3 years

2
2
0 0

[PATCH v6 1/4] panic: Taint kernel if tests are run

by David Gow

Most in-kernel tests (such as KUnit tests) are not supposed to run on production systems: they may do deliberately illegal things to trigger errors, and have security implications (for example, KUnit assertions will often deliberately leak kernel addresses). Add a new taint type, TAINT_TEST to signal that a test has been run. This will be printed as 'N' (originally for kuNit, as every other sensible letter was taken.) This should discourage people from running these tests on production systems, and to make it easier to tell if tests have been run accidentally (by loading the wrong configuration, etc.) Acked-by: Luis Chamberlain <mcgrof(a)kernel.org> Reviewed-by: Brendan Higgins <brendanhiggins(a)google.com> Signed-off-by: David Gow <davidgow(a)google.com> --- This is v6 of the "make tests taint the kernel" patchset. The only changes since v5 (which is the version in linux-next at time of writing) are some rather critical fixes to patch 2/4, where the cruicial check was inverted. (Oops!) The 'N' character for the taint is even less useful now that it's no longer short for kuNit, but all the letters in TEST are taken. :-( No changes since v5: https://lore.kernel.org/linux-kselftest/20220702040959.3232874-1-davidgow@g… No changes since v4: https://lore.kernel.org/linux-kselftest/20220701084744.3002019-1-davidgow@g… Changes since v3: https://lore.kernel.org/lkml/20220513083212.3537869-1-davidgow@google.com/ - Remove the mention of KUnit from the documentation. - Add Luis and Brendan's Acked/Reviewed-by tags. Changes since v2: https://lore.kernel.org/linux-kselftest/20220430030019.803481-1-davidgow@go… - Rename TAINT_KUNIT -> TAINT_TEST. - Split into separate patches for adding the taint, and triggering it. - Taint on a kselftest_module being loaded (patch 3/3) Changes since v1: https://lore.kernel.org/linux-kselftest/20220429043913.626647-1-davidgow@go… - Make the taint per-module, to handle the case when tests are in (longer lasting) modules. (Thanks Greg KH). Note that this still has checkpatch.pl warnings around bracket placement, which are intentional as part of matching the surrounding code. --- Documentation/admin-guide/tainted-kernels.rst | 1 + include/linux/panic.h | 3 ++- kernel/panic.c | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/admin-guide/tainted-kernels.rst b/Documentation/admin-guide/tainted-kernels.rst index ceeed7b0798d..7d80e8c307d1 100644 --- a/Documentation/admin-guide/tainted-kernels.rst +++ b/Documentation/admin-guide/tainted-kernels.rst @@ -100,6 +100,7 @@ Bit Log Number Reason that got the kernel tainted 15 _/K 32768 kernel has been live patched 16 _/X 65536 auxiliary taint, defined for and used by distros 17 _/T 131072 kernel was built with the struct randomization plugin + 18 _/N 262144 an in-kernel test has been run === === ====== ======================================================== Note: The character ``_`` is representing a blank in this table to make reading diff --git a/include/linux/panic.h b/include/linux/panic.h index e71161da69c4..c7759b3f2045 100644 --- a/include/linux/panic.h +++ b/include/linux/panic.h @@ -68,7 +68,8 @@ static inline void set_arch_panic_timeout(int timeout, int arch_default_timeout) #define TAINT_LIVEPATCH 15 #define TAINT_AUX 16 #define TAINT_RANDSTRUCT 17 -#define TAINT_FLAGS_COUNT 18 +#define TAINT_TEST 18 +#define TAINT_FLAGS_COUNT 19 #define TAINT_FLAGS_MAX ((1UL << TAINT_FLAGS_COUNT) - 1) struct taint_flag { diff --git a/kernel/panic.c b/kernel/panic.c index a3c758dba15a..6b3369e21026 100644 --- a/kernel/panic.c +++ b/kernel/panic.c @@ -428,6 +428,7 @@ const struct taint_flag taint_flags[TAINT_FLAGS_COUNT] = { [ TAINT_LIVEPATCH ] = { 'K', ' ', true }, [ TAINT_AUX ] = { 'X', ' ', true }, [ TAINT_RANDSTRUCT ] = { 'T', ' ', true }, + [ TAINT_TEST ] = { 'N', ' ', true }, }; /** -- 2.37.0.rc0.161.g10f37bed90-goog

3 years

3
9
0 0

Re: [PATCH] selftests/proc: Fix proc-pid-vm for vsyscall=xonly.

by Dylan Hatch

Accidentally hit direct reply, adding Shuah Khan <shuah(a)kernel.org>, linux-kernel(a)vger.kernel.org, linux-fsdevel(a)vger.kernel.org, linux-kselftest(a)vger.kernel.org, Shuah Khan <skhan(a)linuxfoundation.org> On Mon, Jul 11, 2022 at 4:04 PM Dylan Hatch <dylanbhatch(a)google.com> wrote: > > On Wed, Jun 22, 2022 at 10:15 AM Shuah Khan <skhan(a)linuxfoundation.org> wrote: > > > > On 6/21/22 6:18 PM, Dylan Hatch wrote: > > > On Fri, Jun 17, 2022 at 3:27 PM Shuah Khan <skhan(a)linuxfoundation.org> wrote: > > >> > > >> On 6/17/22 4:05 PM, Dylan Hatch wrote: > > >>> On Fri, Jun 17, 2022 at 12:38 PM Shuah Khan <skhan(a)linuxfoundation.org> wrote: > > >>>> > > >>>> On 6/17/22 12:45 PM, Dylan Hatch wrote: > > >>>>> On Thu, Jun 16, 2022 at 4:01 PM Shuah Khan <skhan(a)linuxfoundation.org> wrote: > > >>>>>> > > >>> > > >>>> > > >>>> It depends on the goal of the test. Is the test looking to see if the > > >>>> probe fails with insufficient permissions, then you are changing the > > >>>> test to not check for that condition. > > >>> > > >>> The goal of the test is to validate the output of /proc/$PID/maps, and > > >>> the memory probe is only needed as setup to determine what the > > >>> expected output should be. This used to be sufficient, but now it can > > >>> no longer fully disambiguate it with the introduction of > > >>> vsyscall=xonly. The solution proposed here is to disambiguate it by > > >>> also checking the length read from /proc/$PID/maps. > > >>> > > >>>> > > >> > > >> Makes sense. However the question is does this test need to be enhanced > > >> with the addition of vsyscall=xonly? > > >> > > >>>> I would say in this case, the right approach would be to leave the test > > >>>> as is and report expected fail and add other cases. > > >>>> > > >>>> The goal being adding more coverage and not necessarily opt for a simple > > >>>> solution. > > >>> > > >>> What does it mean to report a test as expected fail? Is this a > > >>> mechanism unique to kselftest? I agree adding another test case would > > >>> work, but I'm unsure how to do it within the framework of kselftest. > > >>> Ideally, there would be separate test cases for vsyscall=none, > > >>> vsyscall=emulate, and vsyscall=xonly, but these options can be toggled > > >>> both in the kernel config and on the kernel command line, meaning (to > > >>> the best of my knowledge) these test cases would have to be built > > >>> conditionally against the conflig options and also parse the command > > >>> line for the 'vsyscall' option. > > >>> > > >> > > >> Expected fail isn't unique kselftest. It is a testing criteria where > > >> a test is expected to fail. For example if a file can only be opened > > >> with privileged user a test that runs and looks for failure is an > > >> expected to fail case - we are looking for a failure. > > >> > > >> A complete battery of tests for vsyscall=none, vsyscall=emulate, > > >> vsyscall=xonly would test for conditions that are expected to pass > > >> and fail based on the config. > > >> > > >> tools/testing/selftests/proc/config doesn't have any config options > > >> that are relevant to VSYSCALL > > >> > > >> Can you please send me the how you are running the test and what the > > >> failure output looks like? > > > > > > I'm building a kernel with the following relevant configurations: > > > > > > $ cat .config | grep VSYSCALL > > > CONFIG_GENERIC_TIME_VSYSCALL=y > > > CONFIG_X86_VSYSCALL_EMULATION=y > > > CONFIG_LEGACY_VSYSCALL_XONLY=y > > > # CONFIG_LEGACY_VSYSCALL_NONE is not set > > > > > > Running the test without this change both in virtme and on real > > > hardware gives the following error: > > > > > > # ./tools/testing/selftests/proc/proc-pid-vm > > > proc-pid-vm: proc-pid-vm.c:328: int main(void): Assertion `rv == len' failed. > > > Aborted > > > > > > This is because when CONFIG_LEGACY_VSYSCALL_XONLY=y a probe of the > > > vsyscall page results in a segfault. This test was originally written > > > before this option existed so it incorrectly assumes the vsyscall page > > > isn't mapped at all, and the expected buffer length doesn't match the > > > result. > > > > > > An alternate method of fixing this test could involve setting the > > > expected result based on the config with #ifdef blocks, but I wasn't > > > sure if that could be done for kernel config options in kselftest > > > code. There's also the matter of checking the kernel command line for > > > a `vsyscall=` arg, is parsing /proc/cmdline the best way to do this? > > > > > > > We have a few tests do ifdef to be able to test the code as well as deal > > with config specific tests. Not an issue. > > > > Parsing /proc/cmdline line is flexible for sure, if you want to use that > > route. > > > > Thank you for finding the problem and identifying missing coverage. Look > > forward to any patches fixing the problem. > > > > thanks, > > -- Shuah > I've done some experimenting with ifdefs on config options, but it seems that these options do not propagate properly into the tests. Is there a specific method I should be using to propagate the config values, or would you be able to point me to an example where this is done properly? Thanks and sorry for the slow reply on this, Dylan

3 years

1
0
0 0

[PATCH v3] kunit: tool: Enable virtio/PCI by default on UML

by Daniel Latypov

From: David Gow <davidgow(a)google.com> There are several tests which depend on PCI, and hence need a bunch of extra options to run under UML. This makes it awkward to give configuration instructions (whether in documentation, or as part of a .kunitconfig file), as two separate, incompatible sets of config options are required for UML and "most other architectures". For non-UML architectures, it's possible to add default kconfig options via the qemu_config python files, but there's no equivalent for UML. Add a new tools/testing/kunit/configs/arch_uml.config file containing extra kconfig options to use on UML. Tested-by: José Expósito <jose.exposito89(a)gmail.com> Reviewed-by: Daniel Latypov <dlatypov(a)google.com> Signed-off-by: David Gow <davidgow(a)google.com> Reviewed-by: Brendan Higgins <brendanhiggins(a)google.com> Signed-off-by: Daniel Latypov <dlatypov(a)google.com> --- NOTE: This depends on v4 of the repeatable --kunitconfig patch here: https://patchwork.kernel.org/project/linux-kselftest/patch/20220708013632.1… Please apply it first first. Changes since v2: (dlatypov(a)google.com) - Rebase on top of the -kselftest kunit branch + v4 of the --kunitconfig patch. It rebased cleanly, but it evidently would not apply cleanly due to all the conflicts v4 --kunitconfig had with --qemu_args Changes since v1: https://lore.kernel.org/linux-kselftest/20220624084400.1454579-1-davidgow@g… - (Hopefully) fix a pytype warning re: architecture being None in the tests. (Thanks, Daniel) - Rebase on top of the new combined v3 of the kconfig/kunitconfig patchset. - Add José's Tested-by and Daniel's Reviewed-by. Changes since RFC: https://lore.kernel.org/linux-kselftest/20220622035326.759935-1-davidgow@go… - Rebase on top of the previous kconfig patches. - Fix a missing make_arch_qemuconfig->make_arch_config rename (Thanks Brendan) - Fix the tests to use the base LinuxSourceTreeOperations class, which has no default kconfig options (and so won't conflict with those set in the tests). Only test_build_reconfig_existing_config actually failed, but I updated a few more in case the defaults changed. --- tools/testing/kunit/configs/arch_uml.config | 5 +++++ tools/testing/kunit/kunit_kernel.py | 14 ++++++++++---- tools/testing/kunit/kunit_tool_test.py | 12 ++++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 tools/testing/kunit/configs/arch_uml.config diff --git a/tools/testing/kunit/configs/arch_uml.config b/tools/testing/kunit/configs/arch_uml.config new file mode 100644 index 000000000000..e824ce43b05a --- /dev/null +++ b/tools/testing/kunit/configs/arch_uml.config @@ -0,0 +1,5 @@ +# Config options which are added to UML builds by default + +# Enable virtio/pci, as a lot of tests require it. +CONFIG_VIRTIO_UML=y +CONFIG_UML_PCI_OVER_VIRTIO=y diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index 56492090e28e..f5c26ea89714 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -26,6 +26,7 @@ KUNITCONFIG_PATH = '.kunitconfig' OLD_KUNITCONFIG_PATH = 'last_used_kunitconfig' DEFAULT_KUNITCONFIG_PATH = 'tools/testing/kunit/configs/default.config' BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config' +UML_KCONFIG_PATH = 'tools/testing/kunit/configs/arch_uml.config' OUTFILE_PATH = 'test.log' ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__)) QEMU_CONFIGS_DIR = os.path.join(ABS_TOOL_PATH, 'qemu_configs') @@ -53,7 +54,7 @@ class LinuxSourceTreeOperations: except subprocess.CalledProcessError as e: raise ConfigError(e.output.decode()) - def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig: + def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig: return base_kunitconfig def make_allyesconfig(self, build_dir: str, make_options) -> None: @@ -109,7 +110,7 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations): self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot' self._extra_qemu_params = qemu_arch_params.extra_qemu_params - def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig: + def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig: kconfig = kunit_config.parse_from_string(self._kconfig) kconfig.merge_in_entries(base_kunitconfig) return kconfig @@ -138,6 +139,11 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations): def __init__(self, cross_compile=None): super().__init__(linux_arch='um', cross_compile=cross_compile) + def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig: + kconfig = kunit_config.parse_file(UML_KCONFIG_PATH) + kconfig.merge_in_entries(base_kunitconfig) + return kconfig + def make_allyesconfig(self, build_dir: str, make_options) -> None: stdout.print_with_timestamp( 'Enabling all CONFIGs for UML...') @@ -298,7 +304,7 @@ class LinuxSourceTree: if build_dir and not os.path.exists(build_dir): os.mkdir(build_dir) try: - self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig) + self._kconfig = self._ops.make_arch_config(self._kconfig) self._kconfig.write_to_file(kconfig_path) self._ops.make_olddefconfig(build_dir, make_options) except ConfigError as e: @@ -329,7 +335,7 @@ class LinuxSourceTree: return self.build_config(build_dir, make_options) existing_kconfig = kunit_config.parse_file(kconfig_path) - self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig) + self._kconfig = self._ops.make_arch_config(self._kconfig) if self._kconfig.is_subset_of(existing_kconfig) and not self._kunitconfig_changed(build_dir): return True diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py index ad63d0d34f3f..446ac432d9a4 100755 --- a/tools/testing/kunit/kunit_tool_test.py +++ b/tools/testing/kunit/kunit_tool_test.py @@ -430,6 +430,10 @@ class LinuxSourceTreeTest(unittest.TestCase): f.write('CONFIG_KUNIT=y') tree = kunit_kernel.LinuxSourceTree(build_dir) + # Stub out the source tree operations, so we don't have + # the defaults for any given architecture get in the + # way. + tree._ops = kunit_kernel.LinuxSourceTreeOperations('none', None) mock_build_config = mock.patch.object(tree, 'build_config').start() # Should generate the .config @@ -447,6 +451,10 @@ class LinuxSourceTreeTest(unittest.TestCase): f.write('CONFIG_KUNIT=y\nCONFIG_KUNIT_TEST=y') tree = kunit_kernel.LinuxSourceTree(build_dir) + # Stub out the source tree operations, so we don't have + # the defaults for any given architecture get in the + # way. + tree._ops = kunit_kernel.LinuxSourceTreeOperations('none', None) mock_build_config = mock.patch.object(tree, 'build_config').start() self.assertTrue(tree.build_reconfig(build_dir, make_options=[])) @@ -463,6 +471,10 @@ class LinuxSourceTreeTest(unittest.TestCase): f.write('CONFIG_KUNIT=y\nCONFIG_KUNIT_TEST=y') tree = kunit_kernel.LinuxSourceTree(build_dir) + # Stub out the source tree operations, so we don't have + # the defaults for any given architecture get in the + # way. + tree._ops = kunit_kernel.LinuxSourceTreeOperations('none', None) mock_build_config = mock.patch.object(tree, 'build_config').start() # ... so we should trigger a call to build_config() base-commit: cbb6bc7059151df198b45e883ed731d8f528b65b -- 2.37.0.rc0.161.g10f37bed90-goog

3 years

2
4
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-kselftest-mirror