Created attachment 203217 [details] patch to skip tests if kernel build without CAPABILITIES There are two tests that check if kern.features.security_capability_mode is correctly set (=1) , but if a kernel without CAPABILITIES these tests failed because non existing sysctl. Before patch: NANOBSD# kyua test sys/audit/process-control:cap_enter_success sys/audit/process-control:cap_enter_success -> failed: /var/jenkins/workspace/ocafirmware_build-master/FreeBSD/tests/sys/audit/process-con trol.c:1518: 0 != sysctlbyname(capname, &capinfo, &len, NULL, 0) [0.003s] Results file id is usr_tests.20190328-140620-191316 Results saved to /root/.kyua/store/results.usr_tests.20190328-140620-191316.db 0/1 passed (1 failed) ============== After patch: NANOBSD# kyua test sys/audit/process-control:cap_enter_success sys/audit/process-control:cap_enter_success -> skipped: sysctl kern.features.security_capability_mode doesn't exist [0.003s] Results file id is usr_tests.20190328-181625-015778 Results saved to /root/.kyua/store/results.usr_tests.20190328-181625-015778.db 1/1 passed (0 failed) NANOBSDx# kyua test sys/audit/process-control:cap_getmode_success sys/audit/process-control:cap_getmode_success -> skipped: sysctl kern.features.security_capability_mode doesn't exist [0.003s] Results file id is usr_tests.20190328-181746-138037 Results saved to /root/.kyua/store/results.usr_tests.20190328-181746-138037.db 1/1 passed (0 failed)
This needs to test for errno in {ENOTDIR,ENOENT}.
Created attachment 203222 [details] patch (with errno) to skip tests if kernel build without CAPABILITIES Ok, here is a patch with errno support. But I don't understand what is the purpose of errno here, because it didn't give very useful information ? Output with this -errno version patch applied: # kyua test sys/audit/process-control:cap_enter_success sys/audit/process-control:cap_enter_success -> skipped: sysctl for kern.features.security_capability_mode failed: No such file or directory [0.003s] Results file id is usr_tests.20190328-201847-910402 Results saved to /root/.kyua/store/results.usr_tests.20190328-201847-910402.db 1/1 passed (0 failed) => The 'No such file or directory' seems misleading for me, because the message is about a sysctl.
(In reply to Olivier Cochard from comment #2) Let me rephrase for clarity; the test should be something of the form: if (sysctlbyname(...) == -1) { if (errno == ENOTDIR || errno == ENOENT) { atf_tc_skip("not testing; : %s", capname, strerror(errno)); } else { /* something happened with the sysctlbyname call; report the error */ } } Also, I realize that there's already a macro for this, called `ATF_REQUIRE_FEATURE(..)`. If you change the call to `ATF_REQUIRE_FEATURE("security_capability_mode");`, it will "do the right thing". You'll need to add... `#include <freebsd_test_suite/macros.h>` ... to the source file and also add `CFLAGS+= -I${SRCTOP}/tests` to the Makefile.
Created attachment 203223 [details] patch to skip tests if kernel build without CAPABILITIES Thanks for the tips! The code a lot's cleaner now and the error message too: # kyua test sys/audit/process-control:cap_enter_success sys/audit/process-control:cap_enter_success -> skipped: kernel feature (security_capability_mode) not present [0.003s] Results file id is usr_tests.20190328-204336-320096 Results saved to /root/.kyua/store/results.usr_tests.20190328-204336-320096.db 1/1 passed (0 failed)
LGTM!
Actually, I take that back. You should remove the old sysctlbyname(3) call and only use ATF_REQUIRE_FEATURE.
Created attachment 203226 [details] patch using ATF_REQUIRE_SYSCTL_INT Good remark. The original code was: ``` const char *capname = "kern.features.security_capability_mode"; ATF_REQUIRE_EQ(0, sysctlbyname(capname, &capinfo, &len, NULL, 0)) /* Without CAPABILITY_MODE enabled, cap_enter() returns ENOSYS */ if (!capinfo) atf_tc_skip("Capsicum is not enabled in the system"); ``` So, the purpose was to check if kern.features.security_capability_mode==1. Which mean "Check if capability is enabled". My patch was adding a "Check if this capability exist", before to check its status. If I remove the old sysctlbyname(3) call, I will not catch this condition: - Capability exist - But capability is disabled So I've read freebsd_test_suite/macros.h, and I've found these functions: - ATF_REQUIRE_FEATURE(_feature_name) - ATF_REQUIRE_KERNEL_MODULE(_mod_name) - ATF_REQUIRE_SYSCTL_INT(_mib_name, _required_value) And I think the best way should using ATF_REQUIRE_SYSCTL_INT(capname,1) that manage both condition. With this new patch: # kyua test sys/audit/process-control:cap_enter_success sys/audit/process-control:cap_enter_success -> skipped: sysctl for kern.features.security_capability_mode failed: No such file or director y [0.003s] Results file id is usr_tests.20190328-230635-632822 Results saved to /root/.kyua/store/results.usr_tests.20190328-230635-632822.db 1/1 passed (0 failed)
Actually, ATF_REQUIRE_FEATURE is correct, it just isn't obvious. You have to look at the source for feature_present(3) in lib/libc/gen/feature_present.c to see that it does just what you want. It returns true if the sysctl exists and its value is unequal to 0.
(In reply to Alan Somers from comment #8) Bingo :)... $ man feature_present FEATURE_PRESENT(3) FreeBSD Library Functions Manual FEATURE_PRESENT(3) ... DESCRIPTION The feature_present() function provides a method for an application to determine if a specific kernel feature is present in the currently running kernel. The feature argument specifies the name of the feature to check. The feature_present() function will return 1 if the specified feature is present, otherwise it will return 0. If the feature_present() function is not able to determine the presence of feature due to an internal error it will return 0. RETURN VALUES If feature is present then 1 is returned; otherwise 0 is returned.
Created attachment 203241 [details] cleanup the kernel feature check code New version according to your explanations (thanks you for your patience).
If you need a src committer approval, Approved by: emaste
A commit references this bug: Author: olivier Date: Mon Apr 1 14:21:32 UTC 2019 New revision: 345765 URL: https://svnweb.freebsd.org/changeset/base/345765 Log: Fix and simplify code by using ATF_REQUIRE_FEATURE macro PR: 236857 Reviewed by: asomers, ngie Approved by: emaste MFC after: 1 month Sponsored by: Netflix Changes: head/tests/sys/audit/Makefile head/tests/sys/audit/process-control.c
Waiting for MFC
^Triage: committed and MFCed back in 2019.