Changeset View
Standalone View
sys/kern/uipc_shm.c
Show All 27 Lines | |||||
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT | * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT | ||||
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY | * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY | ||||
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF | * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF | ||||
* SUCH DAMAGE. | * SUCH DAMAGE. | ||||
*/ | */ | ||||
/* | /* | ||||
* Support for shared swap-backed anonymous memory objects via | * Support for shared swap-backed anonymous memory objects via | ||||
* shm_open(2) and shm_unlink(2). While most of the implementation is | * shm_open(2), shm_rename(2), and shm_unlink(2). | ||||
* here, vm_mmap.c contains mapping logic changes. | * 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 | * posixshmcontrol(1) allows users to inspect the state of the memory | ||||
* objects. Per-uid swap resource limit controls total amount of | * objects. Per-uid swap resource limit controls total amount of | ||||
* memory that user can consume for anonymous objects, including | * memory that user can consume for anonymous objects, including | ||||
* shared. | * shared. | ||||
*/ | */ | ||||
#include <sys/cdefs.h> | #include <sys/cdefs.h> | ||||
▲ Show 20 Lines • Show All 636 Lines • ▼ Show 20 Lines | #endif | ||||
FREAD | FWRITE); | FREAD | FWRITE); | ||||
if (error) | if (error) | ||||
return (error); | return (error); | ||||
map->sm_shmfd->shm_path = NULL; | map->sm_shmfd->shm_path = NULL; | ||||
LIST_REMOVE(map, sm_link); | LIST_REMOVE(map, sm_link); | ||||
shm_drop(map->sm_shmfd); | shm_drop(map->sm_shmfd); | ||||
free(map->sm_path, M_SHMFD); | free(map->sm_path, M_SHMFD); | ||||
free(map, M_SHMFD); | free(map, M_SHMFD); | ||||
return (0); | return (0); | ||||
jilles: This adds some additional complication (boolean) to avoid two refcount operations in the rename… | |||||
Done Inline ActionsIt also avoids some cleanup like mtx_destroy that would invalidate the object's state. So: avoiding the shm_drop is best. matthew.bryan_isilon.com: It also avoids some cleanup like mtx_destroy that would invalidate the object's state. So… | |||||
Done Inline ActionsDerp; let's just shm_hold before shm_remove to prevent any invalidation. Fixed. matthew.bryan_isilon.com: Derp; let's just shm_hold before shm_remove to prevent any invalidation. Fixed. | |||||
} | } | ||||
} | } | ||||
return (ENOENT); | return (ENOENT); | ||||
} | } | ||||
int | int | ||||
kern_shm_open(struct thread *td, const char *userpath, int flags, mode_t mode, | kern_shm_open(struct thread *td, const char *userpath, int flags, mode_t mode, | ||||
▲ Show 20 Lines • Show All 180 Lines • ▼ Show 20 Lines | |||||
#endif | #endif | ||||
AUDIT_ARG_UPATH1_CANON(path); | AUDIT_ARG_UPATH1_CANON(path); | ||||
fnv = fnv_32_str(path, FNV1_32_INIT); | fnv = fnv_32_str(path, FNV1_32_INIT); | ||||
sx_xlock(&shm_dict_lock); | sx_xlock(&shm_dict_lock); | ||||
error = shm_remove(path, fnv, td->td_ucred); | error = shm_remove(path, fnv, td->td_ucred); | ||||
sx_xunlock(&shm_dict_lock); | sx_xunlock(&shm_dict_lock); | ||||
free(path, M_TEMP); | free(path, M_TEMP); | ||||
return (error); | |||||
} | |||||
int | |||||
sys_shm_rename(struct thread *td, struct shm_rename_args *uap) | |||||
Done Inline ActionsAre you planning on adding the instrumentation to audit the syscall arguments in a future revision? csjp: Are you planning on adding the instrumentation to audit the syscall arguments in a future… | |||||
Done Inline ActionsLooks like you found the OpenBSM PR :) Yes; my plan is:
matthew.bryan_isilon.com: Looks like you found the OpenBSM PR :)
https://github.com/openbsm/openbsm/pull/54
Yes; my plan… | |||||
{ | |||||
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; | |||||
Done Inline ActionsPlease add something like if ((flags & ~(SHM_RENAME_NOREPLACE | SHM_RENAME_EXCHANGE)) != 0) return (EINVAL); so new flags can be safely added later. jilles: Please add something like
```
if ((flags & ~(SHM_RENAME_NOREPLACE | SHM_RENAME_EXCHANGE)) != 0)… | |||||
/* | |||||
* 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; | |||||
Done Inline ActionsThis comment now applies to both paths. jilles: This comment now applies to both paths. | |||||
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; | |||||
Done Inline ActionsThis case is clearly possible since permissions to rename/unlink a shared memory object are determined by bits on the object itself and may differ between two objects. Since shm_dict_lock is not unlocked in the meantime, the temporary removal should not be visible to other threads. jilles: This case is clearly possible since permissions to rename/unlink a shared memory object are… | |||||
Done Inline ActionsSounds right. I'll clean up the comment. matthew.bryan_isilon.com: Sounds right. I'll clean up the comment. | |||||
} | |||||
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); | return(error); | ||||
} | } | ||||
int | int | ||||
shm_mmap(struct file *fp, vm_map_t map, vm_offset_t *addr, vm_size_t objsize, | shm_mmap(struct file *fp, vm_map_t map, vm_offset_t *addr, vm_size_t objsize, | ||||
vm_prot_t prot, vm_prot_t cap_maxprot, int flags, | vm_prot_t prot, vm_prot_t cap_maxprot, int flags, | ||||
vm_ooffset_t foff, struct thread *td) | vm_ooffset_t foff, struct thread *td) | ||||
{ | { | ||||
struct shmfd *shmfd; | struct shmfd *shmfd; | ||||
▲ Show 20 Lines • Show All 300 Lines • Show Last 20 Lines |
This adds some additional complication (boolean) to avoid two refcount operations in the rename path. I can't judge whether it's worth it.
It still does two hash lookups. Consider returning the shmfd via a struct shmfd ** parameter that can be NULL.