From cdcffc11ed63b064cde40e86992f3cce5a494d5b Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Tue, 6 Aug 2024 18:17:09 +0100 Subject: [PATCH 1/3] * shmem/unix/shm.c (APR_PERMS_SET_IMPLEMENT): Implement for the POSIX shm case and "classic mmap" case. * test/testshm.c (test_named_perms): Add test case. CI: Add tests for various SHM configurations, and that the decision logic is setting the correct APR_USE_SHM_* variables. --- .github/workflows/linux.yml | 8 ++++++++ shmem/unix/shm.c | 34 ++++++++++++++++++++++++++++++++++ test/testshm.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+) diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 9b322f216e..75c22ed8d4 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -17,10 +17,18 @@ jobs: matrix: include: - name: Default + # Check default shm decision logic for Linux: + config-output: APR_USE_SHMEM_MMAP_SHM APR_USE_SHMEM_MMAP_ANON - name: Static config: --enable-static - name: Maintainer-mode config: --enable-maintainer-mode + - name: Named SHM - SysV, Maintainer-mode + config: --enable-maintainer-mode --enable-sysv-shm + config-output: APR_USE_SHMEM_SHMGET + - name: Named SHM - Classic mmap, Maintainer-mode + config: --enable-maintainer-mode ac_cv_func_shm_open=no ac_cv_func_shmget=no + config-output: APR_USE_SHMEM_MMAP_TMP - name: Pool-debug config: --enable-pool-debug - name: Pool-debug, maintainer-mode diff --git a/shmem/unix/shm.c b/shmem/unix/shm.c index 55f4cfb5e6..3ee8b070ea 100644 --- a/shmem/unix/shm.c +++ b/shmem/unix/shm.c @@ -705,6 +705,40 @@ APR_PERMS_SET_IMPLEMENT(shm) if (shmctl(shmid, IPC_SET, &shmbuf) == -1) { return errno; } + return APR_SUCCESS; +#elif APR_USE_SHMEM_MMAP_SHM + apr_shm_t *shm = (apr_shm_t *)theshm; + const char *shm_name; + int fd; + + if (!shm->filename) + return APR_ENOTIMPL; + + shm_name = make_shm_open_safe_name(shm->filename, shm->pool); + + fd = shm_open(shm_name, O_RDWR, 0); + if (fd == -1) + return errno; + + if (fchown(fd, uid, gid)) + return errno; + + if (fchmod(fd, apr_unix_perms2mode(perms))) + return errno; + + return APR_SUCCESS; +#elif APR_USE_SHMEM_MMAP_TMP + apr_shm_t *shm = (apr_shm_t *)theshm; + + if (!shm->filename) + return APR_ENOTIMPL; + + if (chown(shm->filename, uid, gid)) + return errno; + + if (chmod(shm->filename, apr_unix_perms2mode(perms))) + return errno; + return APR_SUCCESS; #else return APR_ENOTIMPL; diff --git a/test/testshm.c b/test/testshm.c index c04ec18482..d17a28c883 100644 --- a/test/testshm.c +++ b/test/testshm.c @@ -285,6 +285,36 @@ static void test_named_delete(abts_case *tc, void *data) ABTS_TRUE(tc, rv != 0); } +static void test_named_perms(abts_case *tc, void *data) +{ + apr_status_t rv; + apr_shm_t *shm; + apr_uid_t uid; + apr_gid_t gid; + + apr_shm_remove(SHARED_FILENAME, p); + + rv = apr_shm_create(&shm, SHARED_SIZE, SHARED_FILENAME, p); + APR_ASSERT_SUCCESS(tc, "Error allocating shared memory block", rv); + if (rv != APR_SUCCESS) { + return; + } + ABTS_PTR_NOTNULL(tc, shm); + + rv = apr_uid_current(&uid, &gid, p); + APR_ASSERT_SUCCESS(tc, "retrieve current uid/gid", rv); + if (rv) return; + + rv = apr_shm_perms_set(shm, + APR_FPROT_UREAD|APR_FPROT_UWRITE, uid, gid); + apr_shm_destroy(shm); + + if (rv == APR_ENOTIMPL) + ABTS_SKIP(tc, data, "apr_shm_perms_set not implemented for named shm"); + else + APR_ASSERT_SUCCESS(tc, "Could not change permissions of shm segment", rv); +} + #endif abts_suite *testshm(abts_suite *suite) @@ -301,6 +331,7 @@ abts_suite *testshm(abts_suite *suite) abts_run_test(suite, test_named, NULL); abts_run_test(suite, test_named_remove, NULL); abts_run_test(suite, test_named_delete, NULL); + abts_run_test(suite, test_named_perms, NULL); #endif return suite; From ca12f67eaf0e8e0a025fbaf0790e30fac37340d3 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Tue, 6 Aug 2024 21:51:57 +0100 Subject: [PATCH 2/3] Don't leak fd on error cases. --- shmem/unix/shm.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/shmem/unix/shm.c b/shmem/unix/shm.c index 3ee8b070ea..5f54a8d9e5 100644 --- a/shmem/unix/shm.c +++ b/shmem/unix/shm.c @@ -710,6 +710,7 @@ APR_PERMS_SET_IMPLEMENT(shm) apr_shm_t *shm = (apr_shm_t *)theshm; const char *shm_name; int fd; + apr_status_t rv; if (!shm->filename) return APR_ENOTIMPL; @@ -720,11 +721,17 @@ APR_PERMS_SET_IMPLEMENT(shm) if (fd == -1) return errno; - if (fchown(fd, uid, gid)) - return errno; + if (fchown(fd, uid, gid)) { + rv = errno; + close(fd); + return rv; + } - if (fchmod(fd, apr_unix_perms2mode(perms))) - return errno; + if (fchmod(fd, apr_unix_perms2mode(perms))) { + rv = errno; + close(fd); + return rv; + } return APR_SUCCESS; #elif APR_USE_SHMEM_MMAP_TMP From cd56995ec187f60596568c554dc5c701e77f119b Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Wed, 7 Aug 2024 09:51:05 +0100 Subject: [PATCH 3/3] Update shmem/unix/shm.c Co-authored-by: Ruediger Pluem --- shmem/unix/shm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shmem/unix/shm.c b/shmem/unix/shm.c index 5f54a8d9e5..ab55abce39 100644 --- a/shmem/unix/shm.c +++ b/shmem/unix/shm.c @@ -732,7 +732,7 @@ APR_PERMS_SET_IMPLEMENT(shm) close(fd); return rv; } - + close(fd); return APR_SUCCESS; #elif APR_USE_SHMEM_MMAP_TMP apr_shm_t *shm = (apr_shm_t *)theshm;