linux-kernel - [PATCH 1/2] ipc/shm: Fix shm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20171130061224.25466-2-philippe.mikoyan@skat.systems>
Date:   Thu, 30 Nov 2017 09:12:23 +0300
From:   Philippe Mikoyan <philippe.mikoyan@...t.systems>
To:     akpm@...ux-foundation.org
Cc:     viro@...iv.linux.org.uk, manfred@...orfullife.com,
        linux-kernel@...r.kernel.org, edgar.kaziakhmedov@...tuozzo.com,
        philippe.mikoyan@...t.systems
Subject: [PATCH 1/2] ipc/shm: Fix shm_nattch incorrect value

This patch fixes that do_shmat increases shm_nattch value twice.

E.g. if memory segment was created just now and process attaches it,
shmctl(..IPC_STAT..) of concurrently running process can at some
point of time return data structure with 'shm_nattch' equal to 2.

Signed-off-by: Philippe Mikoyan <philippe.mikoyan@...t.systems>
Signed-off-by: Edgar Kaziakhmedov <edgar.kaziakhmedov@...tuozzo.com>
---
 ipc/shm.c | 58 +++++++++++++++++++++++++++-------------------------------
 1 file changed, 27 insertions(+), 31 deletions(-)

diff --git a/ipc/shm.c b/ipc/shm.c
index badac463e2c8..565f17925128 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -190,33 +190,31 @@ static inline void shm_rmid(struct ipc_namespace *ns, struct shmid_kernel *s)
 	ipc_rmid(&shm_ids(ns), &s->shm_perm);
 }

-
-static int __shm_open(struct vm_area_struct *vma)
-{
-	struct file *file = vma->vm_file;
-	struct shm_file_data *sfd = shm_file_data(file);
-	struct shmid_kernel *shp;
-
-	shp = shm_lock(sfd->ns, sfd->id);
-
-	if (IS_ERR(shp))
-		return PTR_ERR(shp);
-
-	shp->shm_atim = ktime_get_real_seconds();
-	shp->shm_lprid = task_tgid_vnr(current);
-	shp->shm_nattch++;
-	shm_unlock(shp);
-	return 0;
-}
-
 /* This is called by fork, once for every shm attach. */
 static void shm_open(struct vm_area_struct *vma)
 {
-	int err = __shm_open(vma);
+	struct file *file = vma->vm_file;
+	struct shm_file_data *sfd = shm_file_data(file);
+	struct shmid_kernel *shp;
+	int err = 0;
+
+	shp = shm_lock(sfd->ns, sfd->id);
+
+	if (IS_ERR(shp)) {
+		err = PTR_ERR(shp);
+		goto warn;
+	}
+
+	shp->shm_atim = ktime_get_real_seconds();
+	shp->shm_lprid = task_tgid_vnr(current);
+	shp->shm_nattch++;
+	shm_unlock(shp);
+
 	/*
 	 * We raced in the idr lookup or with shm_destroy().
 	 * Either way, the ID is busted.
 	 */
+warn:
 	WARN_ON_ONCE(err);
 }

@@ -418,19 +416,10 @@ static int shm_mmap(struct file *file, struct vm_area_struct *vma)
 	struct shm_file_data *sfd = shm_file_data(file);
 	int ret;

-	/*
-	 * In case of remap_file_pages() emulation, the file can represent
-	 * removed IPC ID: propogate shm_lock() error to caller.
-	 */
-	ret = __shm_open(vma);
-	if (ret)
-		return ret;
-
 	ret = call_mmap(sfd->file, vma);
-	if (ret) {
-		shm_close(vma);
+	if (ret)
 		return ret;
-	}
+
 	sfd->vm_ops = vma->vm_ops;
 #ifdef CONFIG_MMU
 	WARN_ON(!sfd->vm_ops->fault);
@@ -944,6 +933,7 @@ static int shmctl_stat(struct ipc_namespace *ns, int shmid,
 	tbuf->shm_cpid	= shp->shm_cprid;
 	tbuf->shm_lpid	= shp->shm_lprid;
 	tbuf->shm_nattch = shp->shm_nattch;
+
 	rcu_read_unlock();
 	return result;

@@ -1351,7 +1341,11 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg,

 	path = shp->shm_file->f_path;
 	path_get(&path);
+
+	shp->shm_atim = ktime_get_real_seconds();
+	shp->shm_lprid = task_tgid_vnr(current);
 	shp->shm_nattch++;
+
 	size = i_size_read(d_inode(path.dentry));
 	ipc_unlock_object(&shp->shm_perm);
 	rcu_read_unlock();
@@ -1411,6 +1405,8 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg,

 out_fput:
 	fput(file);
+	if (!err)
+		goto out;

 out_nattch:
 	down_write(&shm_ids(ns).rwsem);
--
2.11.0