[bpf-next] libbpf: Set MFD_NOEXEC_SEAL when creating memfd

Commit Message

Daniel Xu Dec. 29, 2024, 9:44 p.m. UTC

Since 105ff5339f49 ("mm/memfd: add MFD_NOEXEC_SEAL and MFD_EXEC"), the
kernel has started printing a warning if neither MFD_NOEXEC_SEAL nor
MFD_EXEC is set in memfd_create().

To avoid this warning (and also be more secure), set MFD_NOEXEC_SEAL by
default. But since libbpf can be running on potentially very old
kernels, leave a fallback for kernels without MFD_NOEXEC_SEAL support.

Signed-off-by: Daniel Xu <[e-mail]>
---
 tools/lib/bpf/libbpf.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Jiri Olsa Dec. 30, 2024, 4:16 p.m. UTC | #1

On Sun, Dec 29, 2024 at 02:44:33PM -0700, Daniel Xu wrote:
> Since 105ff5339f49 ("mm/memfd: add MFD_NOEXEC_SEAL and MFD_EXEC"), the
> kernel has started printing a warning if neither MFD_NOEXEC_SEAL nor
> MFD_EXEC is set in memfd_create().
> 
> To avoid this warning (and also be more secure), set MFD_NOEXEC_SEAL by
> default. But since libbpf can be running on potentially very old
> kernels, leave a fallback for kernels without MFD_NOEXEC_SEAL support.
> 
> Signed-off-by: Daniel Xu <[e-mail]>

there's similar change posted by Andrei already:
  https://lore.kernel.org/bpf/eTid-pMaxx4d_gMkyFN6fgVGub01RRJYIl1SzTmRG7RtRlPUJOMrVfe2I1W8s0OBHBFy3UN2WGm_e6mak0nGcrZ4ZdxAYRUSDDcUSVMvNA4=@proton.me/T/#u

jirka

> ---
>  tools/lib/bpf/libbpf.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 66173ddb5a2d..46492cc0927d 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1731,12 +1731,24 @@ static int sys_memfd_create(const char *name, unsigned flags)
>  #ifndef MFD_CLOEXEC
>  #define MFD_CLOEXEC 0x0001U
>  #endif
> +#ifndef MFD_NOEXEC_SEAL
> +#define MFD_NOEXEC_SEAL 0x0008U
> +#endif
>  
>  static int create_placeholder_fd(void)
>  {
> +	unsigned int flags = MFD_CLOEXEC | MFD_NOEXEC_SEAL;
> +	const char *name = "libbpf-placeholder-fd";
>  	int fd;
>  
> -	fd = ensure_good_fd(sys_memfd_create("libbpf-placeholder-fd", MFD_CLOEXEC));
> +	fd = ensure_good_fd(sys_memfd_create(name, flags));
> +	if (fd >= 0)
> +		return fd;
> +	else if (errno != EINVAL)
> +		return -errno;
> +
> +	/* Possibly running on kernel without MFD_NOEXEC_SEAL */
> +	fd = ensure_good_fd(sys_memfd_create(name, flags & ~MFD_NOEXEC_SEAL));
>  	if (fd < 0)
>  		return -errno;
>  	return fd;
> -- 
> 2.47.1
>

Alexei Starovoitov Dec. 30, 2024, 7:23 p.m. UTC | #2

On Sun, Dec 29, 2024 at 1:44 PM Daniel Xu <[e-mail]> wrote:
>
> Since 105ff5339f49 ("mm/memfd: add MFD_NOEXEC_SEAL and MFD_EXEC"), the
> kernel has started printing a warning if neither MFD_NOEXEC_SEAL nor
> MFD_EXEC is set in memfd_create().

Except that the code is different now:

        if (!(*flags & (MFD_EXEC | MFD_NOEXEC_SEAL))) {
                if (sysctl >= MEMFD_NOEXEC_SCOPE_NOEXEC_SEAL)
                        *flags |= MFD_NOEXEC_SEAL;
                else
                        *flags |= MFD_EXEC;
        }

        if (!(*flags & MFD_NOEXEC_SEAL) && sysctl >=
MEMFD_NOEXEC_SCOPE_NOEXEC_ENFORCED) {
                pr_err_ratelimited(
                        "%s[%d]: memfd_create() requires
MFD_NOEXEC_SEAL with vm.memfd_noexec=%d\n",
                        current->comm, task_pid_nr(current), sysctl);
                return -EACCES;
        }

Since libbpf doesn't specify either the EXEC or NOEXEC will be
applied automatically depending on the value of sysctl vm.memfd_noexec.
And it will warn only if EXEC flag is used with sysctl == 2.

So the patch helps libbpf avoid the warn on somewhat old kernels,
but not strictly necessary on the new kernels.

This patch is relevant too:
commit 202e14222fad ("memfd: do not -EACCES old memfd_create() users
with vm.memfd_noexec=2")
It has Fixes tag and it should have been backported
to the "somewhat old kernels".

So if the kernel backport process was perfect there would be no kernels
at all where current libbbpf code would cause a warn.

Pls add these details to the commit log and respin.
bpf-next is fine. This isn't really a must-have fix for libbpf,
more nice-to-have behavior.

Patch

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 66173ddb5a2d..46492cc0927d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1731,12 +1731,24 @@  static int sys_memfd_create(const char *name, unsigned flags)
 #ifndef MFD_CLOEXEC
 #define MFD_CLOEXEC 0x0001U
 #endif
+#ifndef MFD_NOEXEC_SEAL
+#define MFD_NOEXEC_SEAL 0x0008U
+#endif
 
 static int create_placeholder_fd(void)
 {
+	unsigned int flags = MFD_CLOEXEC | MFD_NOEXEC_SEAL;
+	const char *name = "libbpf-placeholder-fd";
 	int fd;
 
-	fd = ensure_good_fd(sys_memfd_create("libbpf-placeholder-fd", MFD_CLOEXEC));
+	fd = ensure_good_fd(sys_memfd_create(name, flags));
+	if (fd >= 0)
+		return fd;
+	else if (errno != EINVAL)
+		return -errno;
+
+	/* Possibly running on kernel without MFD_NOEXEC_SEAL */
+	fd = ensure_good_fd(sys_memfd_create(name, flags & ~MFD_NOEXEC_SEAL));
 	if (fd < 0)
 		return -errno;
 	return fd;