On Sun, Oct 22, 2023 at 5:49 PM Yafang Shao <laoar.shao@xxxxxxxxx> wrote:
>
> When we configure the kernel command line with 'mitigations=off' and set
> the sysctl knob 'kernel.unprivileged_bpf_disabled' to 0, the commit
> bc5bc309db45 ("bpf: Inherit system settings for CPU security mitigations")
> causes issues in the execution of 'test_progs -t verifier.' This is because
> 'mitigations=off' bypasses Spectre v1 and Spectre v4 protections.
>
> Currently, when a program requests to run in unprivileged mode
> (kernel.unprivileged_bpf_disabled = 0), the BPF verifier may prevent it
> from running due to the following conditions not being enabled:
>
> - bypass_spec_v1
> - bypass_spec_v4
> - allow_ptr_leaks
> - allow_uninit_stack
>
> While 'mitigations=off' enables the first two conditions, it does not
> enable the latter two. As a result, some test cases in
> 'test_progs -t verifier' that were expected to fail to run may run
> successfully, while others still fail but with different error messages.
> This makes it challenging to address them comprehensively.
>
> Moreover, in the future, we may introduce more fine-grained control over
> CPU mitigations, such as enabling only bypass_spec_v1 or bypass_spec_v4.
>
> Given the complexity of the situation, rather than fixing each broken test
> case individually, it's preferable to skip them when 'mitigations=off' is
> in effect and introduce specific test cases for the new 'mitigations=off'
> scenario. For instance, we can introduce new BTF declaration tags like
> '__failure__nospec', '__failure_nospecv1' and '__failure_nospecv4'.
>
> In this patch, the approach is to simply skip the broken test cases when
> 'mitigations=off' is enabled. The result as follows after this commit,
>
> - without 'mitigations=off'
> - kernel.unprivileged_bpf_disabled = 2
> Summary: 74/948 PASSED, 388 SKIPPED, 0 FAILED
> - kernel.unprivileged_bpf_disabled = 0
> Summary: 74/948 PASSED, 388 SKIPPED, 0 FAILED
> - with 'mitigations=off'
> - kernel.unprivileged_bpf_disabled = 2
> Summary: 74/948 PASSED, 388 SKIPPED, 0 FAILED
> - kernel.unprivileged_bpf_disabled = 0
> Summary: 74/948 PASSED, 388 SKIPPED, 0 FAILED
>
> Fixes: bc5bc309db45 ("bpf: Inherit system settings for CPU security mitigations")
> Reported-by: Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx>
> Closes: https://lore.kernel.org/bpf/CAADnVQKUBJqg+hHtbLeeC2jhoJAWqnmRAzXW3hmUCNSV9kx4sQ@xxxxxxxxxxxxxx
> Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> ---
> tools/testing/selftests/bpf/unpriv_helpers.c | 34 +++++++++++++++++++-
> 1 file changed, 33 insertions(+), 1 deletion(-)
>
> ---
> v1 -> v2: Fix leaked fd
>
> diff --git a/tools/testing/selftests/bpf/unpriv_helpers.c b/tools/testing/selftests/bpf/unpriv_helpers.c
> index 2a6efbd0401e..ca4760795f5d 100644
> --- a/tools/testing/selftests/bpf/unpriv_helpers.c
> +++ b/tools/testing/selftests/bpf/unpriv_helpers.c
> @@ -4,9 +4,41 @@
> #include <stdlib.h>
> #include <error.h>
> #include <stdio.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <fcntl.h>
>
> #include "unpriv_helpers.h"
>
> +static bool get_mitigations_off(void)
> +{
> + char cmdline[4096], *c;
> + int fd, ret = false;
> +
> + fd = open("/proc/cmdline", O_RDONLY);
> + if (fd < 0) {
> + perror("open /proc/cmdline");
> + return false;
> + }
> +
> + if (read(fd, cmdline, sizeof(cmdline) - 1) < 0) {
> + perror("read /proc/cmdline");
> + goto out;
> + }
> +
> + cmdline[sizeof(cmdline) - 1] = '\0';
> + for (c = strtok(cmdline, " \n"); c; c = strtok(NULL, " \n")) {
> + if (!strncmp(c, "mitigtions=off", strlen(c))) {
> + ret = true;
> + break;
> + }
> + }
> +
> +out:
> + close(fd);
> + return ret;
> +}
> +
> bool get_unpriv_disabled(void)
> {
> bool disabled;
> @@ -22,5 +54,5 @@ bool get_unpriv_disabled(void)
> disabled = true;
> }
>
> - return disabled;
> + return disabled ? true : !get_mitigations_off();
> }
> --
> 2.39.3
>
Pls. just igore this wrong patch. Sorry about the noise.
I must be in a sleep state currently. I will send a new one after I
get awake ...
--
Regards
Yafang
