Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linux use MFD_NOEXEC_SEAL for shared memory #23619

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

heitbaum
Copy link
Contributor

Description

linux use MFD_NOEXEC_SEAL for shared memory

ref: https://lore.kernel.org/lkml/20221207154939.2532830-4-jeffxu@google.com/

The new MFD_NOEXEC_SEAL and MFD_EXEC flags allows application to set executable bit at creation time (memfd_create).

When MFD_NOEXEC_SEAL is set, memfd is created without executable bit (mode:0666), and sealed with F_SEAL_EXEC, so it can't be chmod to be executable (mode: 0777) after creation.

When MFD_EXEC flag is set, memfd is created with executable bit
(mode:0777), this is the same as the old behavior of memfd_create.

Motivation and context

Remove warning message from Linux dmesg

How has this been tested?

Build and run tested

What is the effect on users?

No user impact. Removes a warning from Linux dmesg in recent (6.3 and greater) kernels - torvalds/linux@105ff53

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • Student submission (PR was done for educational purposes and will be treated as such)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@@ -15,6 +15,9 @@
#if defined(HAVE_LINUX_MEMFD)
#include <linux/memfd.h>
#include <sys/syscall.h>
#ifndef MFD_NOEXEC_SEAL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should likely be based on a kernel version.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be kernel 6.3, but I assume that some "embedded kernels" would backport it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a small test script to test the results (and subsequently updated the PR to handle both kernels with and without support for MFD_NOEXEC_SEAL - as the kernel that Kodi is compiled under doesn’t necessarily dictate the kernel Kodi runs under)

test code

#include <fcntl.h>
#include <sys/mman.h>
#include <unistd.h>

#include <linux/memfd.h>
#include <stdio.h>
#include <sys/syscall.h>
#ifndef MFD_NOEXEC_SEAL
#define MFD_NOEXEC_SEAL 0x0008U
#endif

int main()
{
  int fd = syscall(SYS_memfd_create, "kodi", MFD_CLOEXEC | MFD_ALLOW_SEALING | MFD_NOEXEC_SEAL);
  printf ("with MFD_NOEXEC_SEAL ret = %d\n", fd);
  if (fd < 0)
  {
    fd = syscall(SYS_memfd_create, "kodi", MFD_CLOEXEC | MFD_ALLOW_SEALING);
    printf ("without MFD_NOEXEC_SEAL ret = %d\n", fd);
  }
  return fd;
}

results:

# uname -r;./test
6.5.0-rc5
with MFD_NOEXEC_SEAL ret = 3

# uname -r;./test
6.1.38
with MFD_NOEXEC_SEAL ret = -1
without MFD_NOEXEC_SEAL ret = 3

Copy link
Member

@fritsch fritsch Aug 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I would still suggest fixing the chroot env. If you are in a proper yocto environment make sure linux-libc-headers are properly created from your target kernel and not from native.

While in general I agree, if the define can be checked, that will include benefits for more users including those with backported kernels.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I would still suggest fixing the chroot env. If you are in a proper yocto environment make sure linux-libc-headers are properly created from your target kernel and not from native.

While in general I agree, if the define can be checked, that will include benefits for more users including those with backported kernels.

👍 am building with LE master (which has the updated headers) - was just addressing other builders that might not have the right headers.

@@ -20,6 +20,10 @@

#include "PlatformDefs.h"

#ifndef MFD_NOEXEC_SEAL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Udmabuf can probably removed because of the superior dma-heaps support.

ref: https://lore.kernel.org/lkml/20221207154939.2532830-4-jeffxu@google.com/

The new MFD_NOEXEC_SEAL and MFD_EXEC flags allows application to set
executable bit at creation time (memfd_create).

When MFD_NOEXEC_SEAL is set, memfd is created without executable bit
(mode:0666), and sealed with F_SEAL_EXEC, so it can't be chmod to be
executable (mode: 0777) after creation.

When MFD_EXEC flag is set, memfd is created with executable bit
(mode:0777), this is the same as the old behavior of memfd_create.

Signed-off-by: Rudi Heitbaum <rudi@heitbaum.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants