Skip to content

Commit

Permalink
common: Mark our memfds as non-executable
Browse files Browse the repository at this point in the history
We only ever use them to store data. Recent Linux kernels now encourage
explicitly declaring whether a memfd is supposed to be executable [1].
This avoids an unsightly warning at boot:

> login: [   85.637785] cockpit-tls[1176]: memfd_create() called without MFD_EXEC or MFD_NOEXEC_SEAL set

Older kernel releases don't know about that flag yet, so add some
ifdeffery and runtime fallback. We need to support building for a new
distro/include files, but running on an older kernel.

[1] https://lwn.net/Articles/918106/
  • Loading branch information
martinpitt committed Jan 9, 2024
1 parent e4dcf6b commit 992c989
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 5 deletions.
9 changes: 8 additions & 1 deletion src/common/cockpitjsonprint.c
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,14 @@ FILE *
cockpit_json_print_open_memfd (const char *name,
int version)
{
int fd = memfd_create ("cockpit login messages", MFD_ALLOW_SEALING | MFD_CLOEXEC);
int fd;
/* current kernels moan about not specifying exec mode */
#ifdef MFD_NOEXEC_SEAL
fd = memfd_create ("cockpit login messages", MFD_ALLOW_SEALING | MFD_CLOEXEC | MFD_NOEXEC_SEAL);
/* fallback for older kernels */
if (fd == -1 && errno == EINVAL)
#endif
fd = memfd_create ("cockpit login messages", MFD_ALLOW_SEALING | MFD_CLOEXEC);
assert (fd != -1);

FILE *stream = fdopen (fd, "w");
Expand Down
22 changes: 18 additions & 4 deletions src/common/test-jsonfds.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,20 @@ typedef struct
char *inaccessible;
} TestFixture;

static int
memfd_create_noexec (const char *name,
unsigned int flags)
{
/* current kernels moan about not specifying exec mode */
#ifdef MFD_NOEXEC_SEAL
int fd = memfd_create (name, flags | MFD_NOEXEC_SEAL);
/* fallback for older kernels */
if (fd != -1 || errno != EINVAL)
return fd;
#endif
return memfd_create (name, flags);
}

static void
test_fixture_setup (TestFixture *fixture,
gconstpointer user_data)
Expand Down Expand Up @@ -428,7 +442,7 @@ test_memfd_error_cases (void)


/* memfd is not properly sealed */
fd = memfd_create ("xyz", MFD_CLOEXEC);
fd = memfd_create_noexec ("xyz", MFD_CLOEXEC);

content = cockpit_memfd_read (fd, &error);
cockpit_assert_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_INVAL, "*incorrect seals set*");
Expand All @@ -437,7 +451,7 @@ test_memfd_error_cases (void)
close (fd);

/* memfd is empty */
fd = memfd_create ("xyz", MFD_ALLOW_SEALING | MFD_CLOEXEC);
fd = memfd_create_noexec ("xyz", MFD_ALLOW_SEALING | MFD_CLOEXEC);
r = fcntl (fd, F_ADD_SEALS, F_SEAL_SHRINK | F_SEAL_GROW | F_SEAL_WRITE);
g_assert (r == 0);

Expand Down Expand Up @@ -533,7 +547,7 @@ test_memfd_json_error_cases (void)
gint r;

/* invalid json */
fd = memfd_create ("xyz", MFD_CLOEXEC | MFD_ALLOW_SEALING);
fd = memfd_create_noexec ("xyz", MFD_CLOEXEC | MFD_ALLOW_SEALING);
g_assert_cmpint (write (fd, "beh", 3), ==, 3);
r = fcntl (fd, F_ADD_SEALS, F_SEAL_SHRINK | F_SEAL_GROW | F_SEAL_WRITE);
g_assert (r == 0);
Expand All @@ -544,7 +558,7 @@ test_memfd_json_error_cases (void)
close (fd);

/* valid json, but not an object */
fd = memfd_create ("xyz", MFD_CLOEXEC | MFD_ALLOW_SEALING);
fd = memfd_create_noexec ("xyz", MFD_CLOEXEC | MFD_ALLOW_SEALING);
g_assert_cmpint (write (fd, "[]", 2), ==, 2);
r = fcntl (fd, F_ADD_SEALS, F_SEAL_SHRINK | F_SEAL_GROW | F_SEAL_WRITE);
g_assert (r == 0);
Expand Down

0 comments on commit 992c989

Please sign in to comment.