All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yhs@fb.com>
To: KP Singh <kpsingh@chromium.org>, James Morris <jmorris@namei.org>,
	<linux-kernel@vger.kernel.org>, <bpf@vger.kernel.org>,
	<linux-security-module@vger.kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Florent Revest <revest@chromium.org>,
	Brendan Jackman <jackmanb@chromium.org>,
	Mimi Zohar <zohar@linux.ibm.com>
Subject: Re: [PATCH bpf-next 3/3] bpf: Update LSM selftests for bpf_ima_inode_hash
Date: Fri, 20 Nov 2020 10:11:51 -0800	[thread overview]
Message-ID: <cad0ea25-8567-368a-1f99-b4adc7440a7f@fb.com> (raw)
In-Reply-To: <20201120131708.3237864-3-kpsingh@chromium.org>



On 11/20/20 5:17 AM, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
> 
> - Update the IMA policy before executing the test binary (this is not an
>    override of the policy, just an append that ensures that hashes are
>    calculated on executions).
> 
> - Call the bpf_ima_inode_hash in the bprm_committed_creds hook and check
>    if the call succeeded and a hash was calculated.
> 
> Signed-off-by: KP Singh <kpsingh@google.com>

LGTM with a few nits below.

Acked-by: Yonghong Song <yhs@fb.com>

> ---
>   tools/testing/selftests/bpf/config            |  3 ++
>   .../selftests/bpf/prog_tests/test_lsm.c       | 32 +++++++++++++++++++
>   tools/testing/selftests/bpf/progs/lsm.c       |  7 +++-
>   3 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
> index 2118e23ac07a..4b5764031368 100644
> --- a/tools/testing/selftests/bpf/config
> +++ b/tools/testing/selftests/bpf/config
> @@ -39,3 +39,6 @@ CONFIG_BPF_JIT=y
>   CONFIG_BPF_LSM=y
>   CONFIG_SECURITY=y
>   CONFIG_LIRC=y
> +CONFIG_IMA=y
> +CONFIG_IMA_WRITE_POLICY=y
> +CONFIG_IMA_READ_POLICY=y
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_lsm.c b/tools/testing/selftests/bpf/prog_tests/test_lsm.c
> index 6ab29226c99b..3f5d64adb233 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_lsm.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_lsm.c
> @@ -52,6 +52,28 @@ int exec_cmd(int *monitored_pid)
>   	return -EINVAL;
>   }
>   
[...]
> +
>   void test_test_lsm(void)
>   {
>   	struct lsm *skel = NULL;
> @@ -66,6 +88,10 @@ void test_test_lsm(void)
>   	if (CHECK(err, "attach", "lsm attach failed: %d\n", err))
>   		goto close_prog;
>   
> +	err = update_ima_policy();
> +	if (CHECK(err != 0, "update_ima_policy", "error = %d\n", err))
> +		goto close_prog;

"err != 0" => err?
"error = %d" => "err %d" for consistency with other usage in this function.

> +
>   	err = exec_cmd(&skel->bss->monitored_pid);
>   	if (CHECK(err < 0, "exec_cmd", "err %d errno %d\n", err, errno))
>   		goto close_prog;
> @@ -83,6 +109,12 @@ void test_test_lsm(void)
>   	CHECK(skel->bss->mprotect_count != 1, "mprotect_count",
>   	      "mprotect_count = %d\n", skel->bss->mprotect_count);
>   
> +	CHECK(skel->data->ima_hash_ret < 0, "ima_hash_ret",
> +	      "ima_hash_ret = %d\n", skel->data->ima_hash_ret);
> +
> +	CHECK(skel->bss->ima_hash == 0, "ima_hash",
> +	      "ima_hash = %lu\n", skel->bss->ima_hash);
> +
>   	syscall(__NR_setdomainname, &buf, -2L);
>   	syscall(__NR_setdomainname, 0, -3L);
>   	syscall(__NR_setdomainname, ~0L, -4L);
> diff --git a/tools/testing/selftests/bpf/progs/lsm.c b/tools/testing/selftests/bpf/progs/lsm.c
> index ff4d343b94b5..b0f9639e4b0a 100644
> --- a/tools/testing/selftests/bpf/progs/lsm.c
> +++ b/tools/testing/selftests/bpf/progs/lsm.c
> @@ -35,6 +35,8 @@ char _license[] SEC("license") = "GPL";
>   int monitored_pid = 0;
>   int mprotect_count = 0;
>   int bprm_count = 0;
> +int ima_hash_ret = -1;

The helper returns type "long", but "int" type here should be fine too.

> +u64 ima_hash = 0;
>   
>   SEC("lsm/file_mprotect")
>   int BPF_PROG(test_int_hook, struct vm_area_struct *vma,
> @@ -65,8 +67,11 @@ int BPF_PROG(test_void_hook, struct linux_binprm *bprm)
>   	__u32 key = 0;
>   	__u64 *value;
>   
> -	if (monitored_pid == pid)
> +	if (monitored_pid == pid) {
>   		bprm_count++;
> +		ima_hash_ret = bpf_ima_inode_hash(bprm->file->f_inode,
> +						  &ima_hash, sizeof(ima_hash));
> +	}
>   
>   	bpf_copy_from_user(args, sizeof(args), (void *)bprm->vma->vm_mm->arg_start);
>   	bpf_copy_from_user(args, sizeof(args), (void *)bprm->mm->arg_start);
> 

  reply	other threads:[~2020-11-20 18:12 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-20 13:17 [PATCH bpf-next 1/3] ima: Implement ima_inode_hash KP Singh
2020-11-20 13:17 ` [PATCH bpf-next 2/3] bpf: Add a BPF helper for getting the IMA hash of an inode KP Singh
2020-11-20 17:47   ` Yonghong Song
2020-11-21  0:14     ` KP Singh
2020-11-24  4:02   ` Alexei Starovoitov
2020-11-24 11:04     ` KP Singh
2020-11-24 15:01       ` KP Singh
2020-11-20 13:17 ` [PATCH bpf-next 3/3] bpf: Update LSM selftests for bpf_ima_inode_hash KP Singh
2020-11-20 18:11   ` Yonghong Song [this message]
2020-11-21  0:20     ` KP Singh
2020-11-20 17:32 ` [PATCH bpf-next 1/3] ima: Implement ima_inode_hash Yonghong Song
2020-11-21  0:08   ` KP Singh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cad0ea25-8567-368a-1f99-b4adc7440a7f@fb.com \
    --to=yhs@fb.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jackmanb@chromium.org \
    --cc=jmorris@namei.org \
    --cc=kpsingh@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=revest@chromium.org \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.