From: Daniel Borkmann <daniel@iogearbox.net>
To: KP Singh <kpsingh@chromium.org>,
linux-kernel@vger.kernel.org, bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
Florent Revest <revest@chromium.org>,
Brendan Jackman <jackmanb@chromium.org>,
Pauline Middelink <middelin@google.com>
Subject: Re: [PATCH bpf-next v3 2/2] bpf: Add tests for bpf_lsm_set_bprm_opts
Date: Tue, 17 Nov 2020 23:47:40 +0100 [thread overview]
Message-ID: <edfe92b9-c97e-b36c-eee1-0fe099d2b596@iogearbox.net> (raw)
In-Reply-To: <20201117021307.1846300-2-kpsingh@chromium.org>
On 11/17/20 3:13 AM, KP Singh wrote:
[...]
> +
> +static int run_set_secureexec(int map_fd, int secureexec)
> +{
> +
^ same here
> + int child_pid, child_status, ret, null_fd;
> +
> + child_pid = fork();
> + if (child_pid == 0) {
> + null_fd = open("/dev/null", O_WRONLY);
> + if (null_fd == -1)
> + exit(errno);
> + dup2(null_fd, STDOUT_FILENO);
> + dup2(null_fd, STDERR_FILENO);
> + close(null_fd);
> +
> + /* Ensure that all executions from hereon are
> + * secure by setting a local storage which is read by
> + * the bprm_creds_for_exec hook and sets bprm->secureexec.
> + */
> + ret = update_storage(map_fd, secureexec);
> + if (ret)
> + exit(ret);
> +
> + /* If the binary is executed with securexec=1, the dynamic
> + * loader ingores and unsets certain variables like LD_PRELOAD,
> + * TMPDIR etc. TMPDIR is used here to simplify the example, as
> + * LD_PRELOAD requires a real .so file.
> + *
> + * If the value of TMPDIR is set, the bash command returns 10
> + * and if the value is unset, it returns 20.
> + */
> + execle("/bin/bash", "bash", "-c",
> + "[[ -z \"${TMPDIR}\" ]] || exit 10 && exit 20", NULL,
> + bash_envp);
> + exit(errno);
> + } else if (child_pid > 0) {
> + waitpid(child_pid, &child_status, 0);
> + ret = WEXITSTATUS(child_status);
> +
> + /* If a secureexec occured, the exit status should be 20.
> + */
> + if (secureexec && ret == 20)
> + return 0;
> +
> + /* If normal execution happened the exit code should be 10.
> + */
> + if (!secureexec && ret == 10)
> + return 0;
> +
and here (rest looks good to me)
> + }
> +
> + return -EINVAL;
> +}
> +
next prev parent reply other threads:[~2020-11-17 22:47 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-17 2:13 [PATCH bpf-next v3 1/2] bpf: Add bpf_lsm_set_bprm_opts helper KP Singh
2020-11-17 2:13 ` [PATCH bpf-next v3 2/2] bpf: Add tests for bpf_lsm_set_bprm_opts KP Singh
2020-11-17 6:18 ` Martin KaFai Lau
2020-11-17 22:47 ` Daniel Borkmann [this message]
2020-11-17 6:14 ` [PATCH bpf-next v3 1/2] bpf: Add bpf_lsm_set_bprm_opts helper Martin KaFai Lau
2020-11-17 22:41 ` Daniel Borkmann
2020-11-17 23:35 ` 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=edfe92b9-c97e-b36c-eee1-0fe099d2b596@iogearbox.net \
--to=daniel@iogearbox.net \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=jackmanb@chromium.org \
--cc=kpsingh@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=middelin@google.com \
--cc=revest@chromium.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).