Re: [PATCH bpf-next v2 4/4] bpf: Add selftests for local_storage

From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: KP Singh <kpsingh@chromium.org>
Cc: bpf <bpf@vger.kernel.org>,
	linux-security-module@vger.kernel.org,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Paul Turner <pjt@google.com>, Jann Horn <jannh@google.com>
Subject: Re: [PATCH bpf-next v2 4/4] bpf: Add selftests for local_storage
Date: Thu, 18 Jun 2020 11:16:37 -0700	[thread overview]
Message-ID: <CAEf4BzZdUWUSLzT1Y-o1Yvy3tTETkJEVU7RyZufZY_yEKzwOSg@mail.gmail.com> (raw)
In-Reply-To: <20200617202941.3034-5-kpsingh@chromium.org>

On Wed, Jun 17, 2020 at 1:31 PM KP Singh <kpsingh@chromium.org> wrote:
>
> From: KP Singh <kpsingh@google.com>
>
> inode_local_storage:
>
> * Hook to the file_open and inode_unlink LSM hooks.
> * Create and unlink a temporary file.
> * Store some information in the inode's bpf_local_storage during
>   file_open.
> * Verify that this information exists when the file is unlinked.
>
> sk_local_storage:
>
> * Hook to the socket_post_create and socket_bind LSM hooks.
> * Open and bind a socket and set the sk_storage in the
>   socket_post_create hook using the start_server helper.
> * Verify if the information is set in the socket_bind hook.
>
> Signed-off-by: KP Singh <kpsingh@google.com>
> ---
>  .../bpf/prog_tests/test_local_storage.c       |  60 ++++++++
>  .../selftests/bpf/progs/local_storage.c       | 137 ++++++++++++++++++
>  2 files changed, 197 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/test_local_storage.c
>  create mode 100644 tools/testing/selftests/bpf/progs/local_storage.c
>

[...]

> diff --git a/tools/testing/selftests/bpf/progs/local_storage.c b/tools/testing/selftests/bpf/progs/local_storage.c
> new file mode 100644
> index 000000000000..38954e6a1edc
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/local_storage.c
> @@ -0,0 +1,137 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright 2020 Google LLC.
> + */
> +
> +#include <errno.h>
> +#include <linux/bpf.h>
> +#include <stdbool.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +char _license[] SEC("license") = "GPL";
> +__u32 _version SEC("version") = 1;

version is anachronism, please drop it.

Otherwise, LGTM.

Acked-by: Andrii Nakryiko <andriin@fb.com>

[...]