Date: Thu, 26 Sep 2019 10:54:23 -0500 From: Kyle Evans <kevans@freebsd.org> To: David Bright <dab@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org> Subject: Re: svn commit: r352747 - in head: lib/libc/sys sys/compat/freebsd32 sys/kern sys/sys tests/sys/posixshm usr.bin/truss Message-ID: <CACNAnaHLd6Mw%2BO83dC7PEJRBkogTdGrEXQhMQHA62CECnCBzTA@mail.gmail.com> In-Reply-To: <201909261532.x8QFWSDH049691@repo.freebsd.org> References: <201909261532.x8QFWSDH049691@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Sep 26, 2019 at 10:32 AM David Bright <dab@freebsd.org> wrote: > > Author: dab > Date: Thu Sep 26 15:32:28 2019 > New Revision: 352747 > URL: https://svnweb.freebsd.org/changeset/base/352747 > > Log: > Add an shm_rename syscall > > Add an atomic shm rename operation, similar in spirit to a file > rename. Atomically unlink an shm from a source path and link it to a > destination path. If an existing shm is linked at the destination > path, unlink it as part of the same atomic operation. The caller needs > the same permissions as shm_unlink to the shm being renamed, and the > same permissions for the shm at the destination which is being > unlinked, if it exists. If those fail, EACCES is returned, as with the > other shm_* syscalls. > > truss support is included; audit support will come later. > > This commit includes only the implementation; the sysent-generated > bits will come in a follow-on commit. > > Submitted by: Matthew Bryan <matthew.bryan@isilon.com> > Reviewed by: jilles (earlier revision) > Reviewed by: brueffer (manpages, earlier revision) > Relnotes: yes > Sponsored by: Dell EMC Isilon > Differential Revision: https://reviews.freebsd.org/D21423 > > Modified: > head/lib/libc/sys/Makefile.inc > head/lib/libc/sys/Symbol.map > head/lib/libc/sys/shm_open.2 > head/sys/compat/freebsd32/syscalls.master > head/sys/kern/syscalls.master > head/sys/kern/uipc_shm.c > head/sys/sys/mman.h > head/tests/sys/posixshm/posixshm_test.c > head/usr.bin/truss/syscalls.c > > Modified: head/lib/libc/sys/Makefile.inc > ============================================================================== > --- head/lib/libc/sys/Makefile.inc Thu Sep 26 15:18:57 2019 (r352746) > +++ head/lib/libc/sys/Makefile.inc Thu Sep 26 15:32:28 2019 (r352747) > @@ -477,7 +477,8 @@ MLINKS+=setuid.2 setegid.2 \ > 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 \ > > Modified: head/lib/libc/sys/Symbol.map > ============================================================================== > --- head/lib/libc/sys/Symbol.map Thu Sep 26 15:18:57 2019 (r352746) > +++ head/lib/libc/sys/Symbol.map Thu Sep 26 15:32:28 2019 (r352747) > @@ -410,6 +410,7 @@ FBSD_1.6 { > getfhat; > funlinkat; > memfd_create; > + shm_rename; > }; > > FBSDprivate_1.0 { > > Modified: head/lib/libc/sys/shm_open.2 > ============================================================================== > --- head/lib/libc/sys/shm_open.2 Thu Sep 26 15:18:57 2019 (r352746) > +++ head/lib/libc/sys/shm_open.2 Thu Sep 26 15:32:28 2019 (r352747) > @@ -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 @@ see > 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 @@ argument to > 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 @@ will fail with > 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 @@ and > .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 @@ The > .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 @@ are specified and the named shared memory object does > 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 @@ The > .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 @@ functions first appeared in > 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 > > Modified: head/sys/compat/freebsd32/syscalls.master > ============================================================================== > --- head/sys/compat/freebsd32/syscalls.master Thu Sep 26 15:18:57 2019 (r352746) > +++ head/sys/compat/freebsd32/syscalls.master Thu Sep 26 15:32:28 2019 (r352747) > @@ -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 > > Modified: head/sys/kern/syscalls.master > ============================================================================== > --- head/sys/kern/syscalls.master Thu Sep 26 15:18:57 2019 (r352746) > +++ head/sys/kern/syscalls.master Thu Sep 26 15:32:28 2019 (r352747) > @@ -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 > > Modified: head/sys/kern/uipc_shm.c > ============================================================================== > --- head/sys/kern/uipc_shm.c Thu Sep 26 15:18:57 2019 (r352746) > +++ head/sys/kern/uipc_shm.c Thu Sep 26 15:32:28 2019 (r352747) > @@ -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 @@ sys_shm_unlink(struct thread *td, struct shm_unlink_ar > 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 > > Modified: head/sys/sys/mman.h > ============================================================================== > --- head/sys/sys/mman.h Thu Sep 26 15:18:57 2019 (r352746) > +++ head/sys/sys/mman.h Thu Sep 26 15:32:28 2019 (r352747) > @@ -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 posix_madvise(void *, size_t, int); > 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 > I think this should also be under __BSD_VISIBLE rather than __POSIX_VISIBLE since it's not specified in any POSIX publication (that I've found).
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACNAnaHLd6Mw%2BO83dC7PEJRBkogTdGrEXQhMQHA62CECnCBzTA>