lkml.org 
[lkml]   [2015]   [Nov]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH, RESEND] ipc/shm: handle removed segments gracefully in shm_mmap()
On Fri, 13 Nov 2015, Bueso wrote:


>So considering EINVAL, even your approach to bumping up nattach by calling
>_shm_open earlier isn't enough. Races exposed to user called rmid can still
>occur between dropping the lock and doing ->mmap(). Ultimately this leads to
>all ipc_valid_object() checks, as we totally ignore SHM_DEST segments nowadays
>since we forbid mapping previously removed segments.
>
>I think this is the first thing we must decide before going forward with this
>mess. ipc currently defines invalid objects by merely checking the deleted flag.

Particularly something like this, which we could then add to the vma validity
check, thus saving the lookup as well.

diff --git a/ipc/shm.c b/ipc/shm.c
index 4178727..d9b2fb1 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -64,6 +64,20 @@ static const struct vm_operations_struct shm_vm_ops;
#define shm_unlock(shp) \
ipc_unlock(&(shp)->shm_perm)

+/*
+ * shm object validity is different than the rest of ipc
+ * as shm needs to deal with segments previously marked
+ * for deletion, which can occur at any time via user calls.
+ */
+static inline int shm_invalid_object(struct kern_ipc_perm *perm)
+{
+ if (perm->mode & SHM_DEST)
+ return -EINVAL;
+ if (ipc_valid_object(perm))
+ return -EIDRM;
+ return 0; /* yay */
+}
+
static int newseg(struct ipc_namespace *, struct ipc_params *);
static void shm_open(struct vm_area_struct *vma);
static void shm_close(struct vm_area_struct *vma);
@@ -985,11 +999,9 @@ SYSCALL_DEFINE3(shmctl, int, shmid, int, cmd, struct shmid_ds __user *, buf)

ipc_lock_object(&shp->shm_perm);

- /* check if shm_destroy() is tearing down shp */
- if (!ipc_valid_object(&shp->shm_perm)) {
- err = -EIDRM;
+ err = shm_invalid_object(&shp->shm_perm);
+ if (err)
goto out_unlock0;
- }

if (!ns_capable(ns->user_ns, CAP_IPC_LOCK)) {
kuid_t euid = current_euid();
@@ -1124,10 +1136,9 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr,

ipc_lock_object(&shp->shm_perm);

- /* check if shm_destroy() is tearing down shp */
- if (!ipc_valid_object(&shp->shm_perm)) {
+ err = shm_invalid_object(&shp->shm_perm);
+ if (err) {
ipc_unlock_object(&shp->shm_perm);
- err = -EIDRM;
goto out_unlock;
}



\
 
 \ /
  Last update: 2015-11-13 21:21    [W:0.061 / U:0.396 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site