Index: head/lib/libc/sys/Makefile.inc =================================================================== --- head/lib/libc/sys/Makefile.inc +++ head/lib/libc/sys/Makefile.inc @@ -477,7 +477,8 @@ setuid.2 setgid.2 MLINKS+=shmat.2 shmdt.2 MLINKS+=shm_open.2 memfd_create.3 \ - shm_open.2 shm_unlink.2 + shm_open.2 shm_unlink.2 \ + shm_rename.2 MLINKS+=sigwaitinfo.2 sigtimedwait.2 MLINKS+=stat.2 fstat.2 \ stat.2 fstatat.2 \ Index: head/lib/libc/sys/Symbol.map =================================================================== --- head/lib/libc/sys/Symbol.map +++ head/lib/libc/sys/Symbol.map @@ -410,6 +410,7 @@ getfhat; funlinkat; memfd_create; + shm_rename; }; FBSDprivate_1.0 { Index: head/lib/libc/sys/shm_open.2 =================================================================== --- head/lib/libc/sys/shm_open.2 +++ head/lib/libc/sys/shm_open.2 @@ -28,11 +28,11 @@ .\" .\" $FreeBSD$ .\" -.Dd September 24, 2019 +.Dd September 26, 2019 .Dt SHM_OPEN 2 .Os .Sh NAME -.Nm memfd_create , shm_open , shm_unlink +.Nm memfd_create , shm_open , shm_rename, shm_unlink .Nd "shared memory object operations" .Sh LIBRARY .Lb libc @@ -45,6 +45,8 @@ .Ft int .Fn shm_open "const char *path" "int flags" "mode_t mode" .Ft int +.Fn shm_rename "const char *path_from" "const char *path_to" "int flags" +.Ft int .Fn shm_unlink "const char *path" .Sh DESCRIPTION The @@ -112,8 +114,9 @@ and .Xr fcntl 2 . .Pp -As a FreeBSD extension, -the constant +As a +.Fx +extension, the constant .Dv SHM_ANON may be used for the .Fa path @@ -122,7 +125,9 @@ In this case, an anonymous, unnamed shared memory object is created. Since the object has no name, it cannot be removed via a subsequent call to -.Fn shm_unlink . +.Fn shm_unlink , +or moved with a call to +.Fn shm_rename . Instead, the shared memory object will be garbage collected when the last reference to the shared memory object is removed. @@ -138,6 +143,31 @@ All other flags are ignored. .Pp The +.Fn shm_rename +system call atomically removes a shared memory object named +.Fa path_from +and relinks it at +.Fa path_to . +If another object is already linked at +.Fa path_to , +that object will be unlinked, unless one of the following flags are provided: +.Bl -tag -offset indent -width Er +.It Er SHM_RENAME_EXCHANGE +Atomically exchange the shms at +.Fa path_from +and +.Fa path_to . +.It Er SHM_RENAME_NOREPLACE +Return an error if an shm exists at +.Fa path_to , +rather than unlinking it. +.El +.Fn shm_rename +is also a +.Fx +extension. +.Pp +The .Fn shm_unlink system call removes a shared memory object named .Fa path . @@ -196,15 +226,20 @@ .Fn shm_open both return a non-negative integer, and +.Fn shm_rename +and .Fn shm_unlink -returns zero. -All three functions return -1 on failure, and set +return zero. +All functions return -1 on failure, and set .Va errno to indicate the error. .Sh COMPATIBILITY The -.Fa path -argument does not necessarily represent a pathname (although it does in +.Fa path , +.Fa path_from , +and +.Fa path_to +arguments do not necessarily represent a pathname (although they do in most other implementations). Two processes opening the same .Fa path @@ -325,7 +360,7 @@ .Fa path argument points outside the process' allocated address space. .It Bq Er ENAMETOOLONG -The entire pathname exceeded 1023 characters. +The entire pathname exceeds 1023 characters. .It Bq Er EINVAL The .Fa path @@ -344,6 +379,31 @@ The required permissions (for reading or reading and writing) are denied. .El .Pp +The following errors are defined for +.Fn shm_rename : +.Bl -tag -width Er +.It Bq Er EFAULT +The +.Fa path_from +or +.Fa path_to +argument points outside the process' allocated address space. +.It Bq Er ENAMETOOLONG +The entire pathname exceeds 1023 characters. +.It Bq Er ENOENT +The shared memory object at +.Fa path_from +does not exist. +.It Bq Er EACCES +The required permissions are denied. +.It Bq Er EEXIST +An shm exists at +.Fa path_to , +and the +.Dv SHM_RENAME_NOREPLACE +flag was provided. +.El +.Pp .Fn shm_unlink fails with these error codes for these conditions: .Bl -tag -width Er @@ -352,7 +412,7 @@ .Fa path argument points outside the process' allocated address space. .It Bq Er ENAMETOOLONG -The entire pathname exceeded 1023 characters. +The entire pathname exceeds 1023 characters. .It Bq Er ENOENT The named shared memory object does not exist. .It Bq Er EACCES @@ -394,9 +454,19 @@ The functions were reimplemented as system calls using shared memory objects directly rather than files in .Fx 8.0 . +.Pp +.Fn shm_rename +first appeared in +.Fx 13.0 +as a +.Fx +extension. .Sh AUTHORS .An Garrett A. Wollman Aq Mt wollman@FreeBSD.org (C library support and this manual page) .Pp .An Matthew Dillon Aq Mt dillon@FreeBSD.org .Pq Dv MAP_NOSYNC +.Pp +.An Matthew Bryan Aq Mt matthew.bryan@isilon.com +.Pq Dv shm_rename implementation Index: head/sys/compat/freebsd32/syscalls.master =================================================================== --- head/sys/compat/freebsd32/syscalls.master +++ head/sys/compat/freebsd32/syscalls.master @@ -1157,5 +1157,7 @@ 571 AUE_SHMOPEN NOPROTO { int shm_open2( \ const char *path, int flags, mode_t mode, \ int shmflags, const char *name); } +572 AUE_NULL NOPROTO { int shm_rename(const char *path_from, \ + const char *path_to, int flags); } ; vim: syntax=off Index: head/sys/kern/syscalls.master =================================================================== --- head/sys/kern/syscalls.master +++ head/sys/kern/syscalls.master @@ -3204,6 +3204,13 @@ _In_z_ const char *name ); } +572 AUE_NULL STD { + int shm_rename( + _In_z_ const char *path_from, + _In_z_ const char *path_to, + int flags + ); + } ; Please copy any additions and changes to the following compatability tables: ; sys/compat/freebsd32/syscalls.master Index: head/sys/kern/uipc_shm.c =================================================================== --- head/sys/kern/uipc_shm.c +++ head/sys/kern/uipc_shm.c @@ -33,8 +33,9 @@ /* * Support for shared swap-backed anonymous memory objects via - * shm_open(2) and shm_unlink(2). While most of the implementation is - * here, vm_mmap.c contains mapping logic changes. + * shm_open(2), shm_rename(2), and shm_unlink(2). + * While most of the implementation is here, vm_mmap.c contains + * mapping logic changes. * * posixshmcontrol(1) allows users to inspect the state of the memory * objects. Per-uid swap resource limit controls total amount of @@ -945,6 +946,158 @@ free(path, M_TEMP); return (error); +} + +int +sys_shm_rename(struct thread *td, struct shm_rename_args *uap) +{ + char *path_from = NULL, *path_to = NULL; + Fnv32_t fnv_from, fnv_to; + struct shmfd *fd_from; + struct shmfd *fd_to; + int error; + int flags; + + flags = uap->flags; + + /* + * Make sure the user passed only valid flags. + * If you add a new flag, please add a new term here. + */ + if ((flags & ~( + SHM_RENAME_NOREPLACE | + SHM_RENAME_EXCHANGE + )) != 0) { + error = EINVAL; + goto out; + } + + /* + * EXCHANGE and NOREPLACE don't quite make sense together. Let's + * force the user to choose one or the other. + */ + if ((flags & SHM_RENAME_NOREPLACE) != 0 && + (flags & SHM_RENAME_EXCHANGE) != 0) { + error = EINVAL; + goto out; + } + + /* + * Malloc zone M_SHMFD, since this path may end up freed later from + * M_SHMFD if we end up doing an insert. + */ + path_from = malloc(MAXPATHLEN, M_SHMFD, M_WAITOK); + error = copyinstr(uap->path_from, path_from, MAXPATHLEN, NULL); + if (error) + goto out; + + path_to = malloc(MAXPATHLEN, M_SHMFD, M_WAITOK); + error = copyinstr(uap->path_to, path_to, MAXPATHLEN, NULL); + if (error) + goto out; + + /* Rename with from/to equal is a no-op */ + if (strncmp(path_from, path_to, MAXPATHLEN) == 0) + goto out; + + fnv_from = fnv_32_str(path_from, FNV1_32_INIT); + fnv_to = fnv_32_str(path_to, FNV1_32_INIT); + + sx_xlock(&shm_dict_lock); + + fd_from = shm_lookup(path_from, fnv_from); + if (fd_from == NULL) { + sx_xunlock(&shm_dict_lock); + error = ENOENT; + goto out; + } + + fd_to = shm_lookup(path_to, fnv_to); + if ((flags & SHM_RENAME_NOREPLACE) != 0 && fd_to != NULL) { + sx_xunlock(&shm_dict_lock); + error = EEXIST; + goto out; + } + + /* + * Unconditionally prevents shm_remove from invalidating the 'from' + * shm's state. + */ + shm_hold(fd_from); + error = shm_remove(path_from, fnv_from, td->td_ucred); + + /* + * One of my assumptions failed if ENOENT (e.g. locking didn't + * protect us) + */ + KASSERT(error != ENOENT, ("Our shm disappeared during shm_rename: %s", + path_from)); + if (error) { + shm_drop(fd_from); + sx_xunlock(&shm_dict_lock); + goto out; + } + + /* + * If we are exchanging, we need to ensure the shm_remove below + * doesn't invalidate the dest shm's state. + */ + if ((flags & SHM_RENAME_EXCHANGE) != 0 && fd_to != NULL) + shm_hold(fd_to); + + /* + * NOTE: if path_to is not already in the hash, c'est la vie; + * it simply means we have nothing already at path_to to unlink. + * That is the ENOENT case. + * + * If we somehow don't have access to unlink this guy, but + * did for the shm at path_from, then relink the shm to path_from + * and abort with EACCES. + * + * All other errors: that is weird; let's relink and abort the + * operation. + */ + error = shm_remove(path_to, fnv_to, td->td_ucred); + if (error && error != ENOENT) { + shm_insert(path_from, fnv_from, fd_from); + shm_drop(fd_from); + /* Don't free path_from now, since the hash references it */ + path_from = NULL; + sx_xunlock(&shm_dict_lock); + goto out; + } + + shm_insert(path_to, fnv_to, fd_from); + + /* Don't free path_to now, since the hash references it */ + path_to = NULL; + + /* We kept a ref when we removed, and incremented again in insert */ + shm_drop(fd_from); +#ifdef DEBUG + KASSERT(fd_from->shm_refs > 0, ("Expected >0 refs; got: %d\n", + fd_from->shm_refs)); +#endif + + if ((flags & SHM_RENAME_EXCHANGE) != 0 && fd_to != NULL) { + shm_insert(path_from, fnv_from, fd_to); + path_from = NULL; + shm_drop(fd_to); +#ifdef DEBUG + KASSERT(fd_to->shm_refs > 0, ("Expected >0 refs; got: %d\n", + fd_to->shm_refs)); +#endif + } + + error = 0; + sx_xunlock(&shm_dict_lock); + +out: + if (path_from != NULL) + free(path_from, M_SHMFD); + if (path_to != NULL) + free(path_to, M_SHMFD); + return(error); } int Index: head/sys/sys/mman.h =================================================================== --- head/sys/sys/mman.h +++ head/sys/sys/mman.h @@ -134,6 +134,14 @@ #define MAP_FAILED ((void *)-1) /* + * Flags provided to shm_rename + */ +/* Don't overwrite dest, if it exists */ +#define SHM_RENAME_NOREPLACE (1 << 0) +/* Atomically swap src and dest */ +#define SHM_RENAME_EXCHANGE (1 << 1) + +/* * msync() flags */ #define MS_SYNC 0x0000 /* msync synchronously */ @@ -313,6 +321,7 @@ int mlockall(int); int munlockall(void); int shm_open(const char *, int, mode_t); +int shm_rename(const char *, const char *, int); int shm_unlink(const char *); #endif #if __BSD_VISIBLE Index: head/tests/sys/posixshm/posixshm_test.c =================================================================== --- head/tests/sys/posixshm/posixshm_test.c +++ head/tests/sys/posixshm/posixshm_test.c @@ -34,6 +34,7 @@ #include #include +#include #include #include #include @@ -46,20 +47,36 @@ #define TEST_PATH_LEN 256 static char test_path[TEST_PATH_LEN]; +static char test_path2[TEST_PATH_LEN]; +static unsigned int test_path_idx = 0; static void -gen_test_path(void) +gen_a_test_path(char *path) { + snprintf(path, TEST_PATH_LEN, "%s/tmp.XXXXXX%d", + getenv("TMPDIR") == NULL ? "/tmp" : getenv("TMPDIR"), + test_path_idx); - snprintf(test_path, sizeof(test_path), "%s/tmp.XXXXXX", - getenv("TMPDIR") == NULL ? "/tmp" : getenv("TMPDIR")); - test_path[sizeof(test_path) - 1] = '\0'; - ATF_REQUIRE_MSG(mkstemp(test_path) != -1, + test_path_idx++; + + ATF_REQUIRE_MSG(mkstemp(path) != -1, "mkstemp failed; errno=%d", errno); - ATF_REQUIRE_MSG(unlink(test_path) == 0, + ATF_REQUIRE_MSG(unlink(path) == 0, "unlink failed; errno=%d", errno); } +static void +gen_test_path(void) +{ + gen_a_test_path(test_path); +} + +static void +gen_test_path2(void) +{ + gen_a_test_path(test_path2); +} + /* * Attempt a shm_open() that should fail with an expected error of 'error'. */ @@ -89,20 +106,18 @@ } /* - * Open the test object and write '1' to the first byte. Returns valid fd + * Open the test object and write a value to the first byte. Returns valid fd * on success and -1 on failure. */ static int -scribble_object(void) +scribble_object(const char *path, char value) { char *page; int fd, pagesize; - gen_test_path(); - ATF_REQUIRE(0 < (pagesize = getpagesize())); - fd = shm_open(test_path, O_CREAT|O_EXCL|O_RDWR, 0777); + fd = shm_open(path, O_CREAT|O_EXCL|O_RDWR, 0777); if (fd < 0 && errno == EEXIST) { if (shm_unlink(test_path) < 0) atf_tc_fail("shm_unlink"); @@ -117,13 +132,45 @@ if (page == MAP_FAILED) atf_tc_fail("mmap failed; errno=%d", errno); - page[0] = '1'; + page[0] = value; ATF_REQUIRE_MSG(munmap(page, pagesize) == 0, "munmap failed; errno=%d", errno); return (fd); } +/* + * Fail the test case if the 'path' does not refer to an shm whose first byte + * is equal to expected_value + */ +static void +verify_object(const char *path, char expected_value) +{ + int fd; + int pagesize; + char *page; + + ATF_REQUIRE(0 < (pagesize = getpagesize())); + + fd = shm_open(path, O_RDONLY, 0777); + if (fd < 0) + atf_tc_fail("shm_open failed in verify_object; errno=%d, path=%s", + errno, path); + + page = mmap(0, pagesize, PROT_READ, MAP_SHARED, fd, 0); + if (page == MAP_FAILED) + atf_tc_fail("mmap(1)"); + if (page[0] != expected_value) + atf_tc_fail("Renamed object has incorrect value; has" + "%d (0x%x, '%c'), expected %d (0x%x, '%c')\n", + page[0], page[0], isprint(page[0]) ? page[0] : ' ', + expected_value, expected_value, + isprint(expected_value) ? expected_value : ' '); + ATF_REQUIRE_MSG(munmap(page, pagesize) == 0, "munmap failed; errno=%d", + errno); + close(fd); +} + ATF_TC_WITHOUT_HEAD(remap_object); ATF_TC_BODY(remap_object, tc) { @@ -132,7 +179,8 @@ ATF_REQUIRE(0 < (pagesize = getpagesize())); - fd = scribble_object(); + gen_test_path(); + fd = scribble_object(test_path, '1'); page = mmap(0, pagesize, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); if (page == MAP_FAILED) @@ -149,6 +197,209 @@ "shm_unlink failed; errno=%d", errno); } +ATF_TC_WITHOUT_HEAD(rename_from_anon); +ATF_TC_BODY(rename_from_anon, tc) +{ + int rc; + + gen_test_path(); + rc = shm_rename(SHM_ANON, test_path, 0); + if (rc != -1) + atf_tc_fail("shm_rename from SHM_ANON succeeded unexpectedly"); +} + +ATF_TC_WITHOUT_HEAD(rename_bad_path_pointer); +ATF_TC_BODY(rename_bad_path_pointer, tc) +{ + const char *bad_path; + int rc; + + bad_path = (const char *)0x1; + + gen_test_path(); + rc = shm_rename(test_path, bad_path, 0); + if (rc != -1) + atf_tc_fail("shm_rename of nonexisting shm succeeded unexpectedly"); + + rc = shm_rename(bad_path, test_path, 0); + if (rc != -1) + atf_tc_fail("shm_rename of nonexisting shm succeeded unexpectedly"); +} + +ATF_TC_WITHOUT_HEAD(rename_from_nonexisting); +ATF_TC_BODY(rename_from_nonexisting, tc) +{ + int rc; + + gen_test_path(); + rc = shm_rename(test_path, test_path2, 0); + if (rc != -1) + atf_tc_fail("shm_rename of nonexisting shm succeeded unexpectedly"); + + if (errno != ENOENT) + atf_tc_fail("Expected ENOENT to rename of nonexistent shm"); +} + +ATF_TC_WITHOUT_HEAD(rename_to_anon); +ATF_TC_BODY(rename_to_anon, tc) +{ + int rc; + + gen_test_path(); + rc = shm_rename(test_path, SHM_ANON, 0); + if (rc != -1) + atf_tc_fail("shm_rename to SHM_ANON succeeded unexpectedly"); +} + +ATF_TC_WITHOUT_HEAD(rename_to_replace); +ATF_TC_BODY(rename_to_replace, tc) +{ + char expected_value; + int fd; + int fd2; + + // Some contents we can verify later + expected_value = 'g'; + + gen_test_path(); + fd = scribble_object(test_path, expected_value); + close(fd); + + // Give the other some different value so we can detect success + gen_test_path2(); + fd2 = scribble_object(test_path2, 'h'); + close(fd2); + + ATF_REQUIRE_MSG(shm_rename(test_path, test_path2, 0) == 0, + "shm_rename failed; errno=%d", errno); + + // Read back renamed; verify contents + verify_object(test_path2, expected_value); +} + +ATF_TC_WITHOUT_HEAD(rename_to_noreplace); +ATF_TC_BODY(rename_to_noreplace, tc) +{ + char expected_value_from; + char expected_value_to; + int fd_from; + int fd_to; + int rc; + + // Some contents we can verify later + expected_value_from = 'g'; + gen_test_path(); + fd_from = scribble_object(test_path, expected_value_from); + close(fd_from); + + // Give the other some different value so we can detect success + expected_value_to = 'h'; + gen_test_path2(); + fd_to = scribble_object(test_path2, expected_value_to); + close(fd_to); + + rc = shm_rename(test_path, test_path2, SHM_RENAME_NOREPLACE); + ATF_REQUIRE_MSG((rc == -1) && (errno == EEXIST), + "shm_rename didn't fail as expected; errno: %d; return: %d", errno, + rc); + + // Read back renamed; verify contents + verify_object(test_path2, expected_value_to); +} + +ATF_TC_WITHOUT_HEAD(rename_to_exchange); +ATF_TC_BODY(rename_to_exchange, tc) +{ + char expected_value_from; + char expected_value_to; + int fd_from; + int fd_to; + + // Some contents we can verify later + expected_value_from = 'g'; + gen_test_path(); + fd_from = scribble_object(test_path, expected_value_from); + close(fd_from); + + // Give the other some different value so we can detect success + expected_value_to = 'h'; + gen_test_path2(); + fd_to = scribble_object(test_path2, expected_value_to); + close(fd_to); + + ATF_REQUIRE_MSG(shm_rename(test_path, test_path2, + SHM_RENAME_EXCHANGE) == 0, + "shm_rename failed; errno=%d", errno); + + // Read back renamed; verify contents + verify_object(test_path, expected_value_to); + verify_object(test_path2, expected_value_from); +} + +ATF_TC_WITHOUT_HEAD(rename_to_exchange_nonexisting); +ATF_TC_BODY(rename_to_exchange_nonexisting, tc) +{ + char expected_value_from; + int fd_from; + + // Some contents we can verify later + expected_value_from = 'g'; + gen_test_path(); + fd_from = scribble_object(test_path, expected_value_from); + close(fd_from); + + gen_test_path2(); + + ATF_REQUIRE_MSG(shm_rename(test_path, test_path2, + SHM_RENAME_EXCHANGE) == 0, + "shm_rename failed; errno=%d", errno); + + // Read back renamed; verify contents + verify_object(test_path2, expected_value_from); +} + +ATF_TC_WITHOUT_HEAD(rename_to_self); +ATF_TC_BODY(rename_to_self, tc) +{ + int fd; + char expected_value; + + expected_value = 't'; + + gen_test_path(); + fd = scribble_object(test_path, expected_value); + close(fd); + + ATF_REQUIRE_MSG(shm_rename(test_path, test_path, 0) == 0, + "shm_rename failed; errno=%d", errno); + + verify_object(test_path, expected_value); +} + +ATF_TC_WITHOUT_HEAD(rename_bad_flag); +ATF_TC_BODY(rename_bad_flag, tc) +{ + int fd; + int rc; + + /* Make sure we don't fail out due to ENOENT */ + gen_test_path(); + gen_test_path2(); + fd = scribble_object(test_path, 'd'); + close(fd); + fd = scribble_object(test_path2, 'd'); + close(fd); + + /* + * Note: if we end up with enough flags that we use all the bits, + * then remove this test completely. + */ + rc = shm_rename(test_path, test_path2, INT_MIN); + ATF_REQUIRE_MSG((rc == -1) && (errno == EINVAL), + "shm_rename should have failed with EINVAL; got: return=%d, " + "errno=%d", rc, errno); +} + ATF_TC_WITHOUT_HEAD(reopen_object); ATF_TC_BODY(reopen_object, tc) { @@ -157,7 +408,8 @@ ATF_REQUIRE(0 < (pagesize = getpagesize())); - fd = scribble_object(); + gen_test_path(); + fd = scribble_object(test_path, '1'); close(fd); fd = shm_open(test_path, O_RDONLY, 0777); @@ -634,6 +886,16 @@ { ATF_TP_ADD_TC(tp, remap_object); + ATF_TP_ADD_TC(tp, rename_from_anon); + ATF_TP_ADD_TC(tp, rename_bad_path_pointer); + ATF_TP_ADD_TC(tp, rename_from_nonexisting); + ATF_TP_ADD_TC(tp, rename_to_anon); + ATF_TP_ADD_TC(tp, rename_to_replace); + ATF_TP_ADD_TC(tp, rename_to_noreplace); + ATF_TP_ADD_TC(tp, rename_to_exchange); + ATF_TP_ADD_TC(tp, rename_to_exchange_nonexisting); + ATF_TP_ADD_TC(tp, rename_to_self); + ATF_TP_ADD_TC(tp, rename_bad_flag); ATF_TP_ADD_TC(tp, reopen_object); ATF_TP_ADD_TC(tp, readonly_mmap_write); ATF_TP_ADD_TC(tp, open_after_link); Index: head/usr.bin/truss/syscalls.c =================================================================== --- head/usr.bin/truss/syscalls.c +++ head/usr.bin/truss/syscalls.c @@ -471,6 +471,8 @@ { Ptr | IN, 3 }, { Socklent, 4 } } }, { .name = "shm_open", .ret_type = 1, .nargs = 3, .args = { { ShmName | IN, 0 }, { Open, 1 }, { Octal, 2 } } }, + { .name = "shm_rename", .ret_type = 1, .nargs = 3, + .args = { { Name | IN, 0 }, { Name | IN, 1 }, { Hex, 2 } } }, { .name = "shm_unlink", .ret_type = 1, .nargs = 1, .args = { { Name | IN, 0 } } }, { .name = "shutdown", .ret_type = 1, .nargs = 2,