linux-kernel - Re: [PATCH v4 4/4] selftest/bpf: Test a perf bpf program that suppresses side effects.

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: <CAPhsuW7Qr_sDDDR_PLZsLKHFM+KHScwonKVvprTWVbK=dEZJ4A@mail.gmail.com>
Date: Fri, 19 Jan 2024 09:19:31 -0800
From: Song Liu <song@...nel.org>
To: Kyle Huey <me@...ehuey.com>
Cc: Kyle Huey <khuey@...ehuey.com>, linux-kernel@...r.kernel.org, 
	Andrii Nakryiko <andrii.nakryiko@...il.com>, Jiri Olsa <jolsa@...nel.org>, 
	Namhyung Kim <namhyung@...nel.org>, Marco Elver <elver@...gle.com>, 
	Yonghong Song <yonghong.song@...ux.dev>, "Robert O'Callahan" <robert@...llahan.org>, 
	Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, 
	John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>, 
	Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>, Mykola Lysenko <mykolal@...com>, 
	Shuah Khan <shuah@...nel.org>, bpf@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v4 4/4] selftest/bpf: Test a perf bpf program that
 suppresses side effects.

On Thu, Jan 18, 2024 at 4:14 PM Kyle Huey <me@...ehuey.com> wrote:
>

Acked-by: Song Liu <song@...nel.org>

with a couple nitpicks below.

[...]
> +int sigio_count, sigtrap_count;
> +
> +static void handle_sigio(int sig __always_unused)
> +{
> +       ++sigio_count;
> +}
> +
> +static void handle_sigtrap(int signum __always_unused,
> +                          siginfo_t *info,
> +                          void *ucontext __always_unused)
> +{
> +       ASSERT_EQ(info->si_code, TRAP_PERF, "wrong si_code");

nit: I would just call it "si_code", not "wrong si_code".

> +       ++sigtrap_count;
> +}
[...]
> +       if (!ASSERT_OK(sigaction(SIGTRAP, &action, &previous_sigtrap), "sigaction"))
> +               return;
> +
> +       previous_sigio = signal(SIGIO, handle_sigio);
> +       if (!ASSERT_NEQ(previous_sigio, SIG_ERR, "signal"))
> +               goto cleanup;

nit: If we goto cleanup here, we will do

       signal(SIGIO, SIG_ERR);

This is a no-op, so it is not a real issue. But it is not very clean.

[...]