| 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
| ||
|
Message-Id: <20171202153456.6514-1-philippe.mikoyan@skat.systems>
Date: Sat, 2 Dec 2017 18:34:56 +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, dave@...olabs.net,
philippe.mikoyan@...t.systems
Subject: [PATCH v2] ipc: Fix ipc data structures inconsistency
As described in the title, this patch fixes <ipc>id_ds
inconsistency when <ipc>ctl_stat executes concurrently
with some ds-changing function, e.g. shmat, msgsnd or
whatever.
For instance, if shmctl(IPC_STAT) is running concurrently
with shmat, following data structure can be returned:
{... shm_lpid = 0, shm_nattch = 1, ...}
Signed-off-by: Philippe Mikoyan <philippe.mikoyan@...t.systems>
Reviewed-by: Davidlohr Bueso <dbueso@...e.de>
---
Changes in v2:
- Fixed 'operatoins' typo in util.c
ipc/msg.c | 20 ++++++++++++++------
ipc/sem.c | 10 ++++++++++
ipc/shm.c | 19 ++++++++++++++-----
ipc/util.c | 5 ++++-
4 files changed, 42 insertions(+), 12 deletions(-)
diff --git a/ipc/msg.c b/ipc/msg.c
index 06be5a9adfa4..047579b42de4 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -475,9 +475,9 @@ static int msgctl_info(struct ipc_namespace *ns, int msqid,
static int msgctl_stat(struct ipc_namespace *ns, int msqid,
int cmd, struct msqid64_ds *p)
{
- int err;
struct msg_queue *msq;
- int success_return;
+ int id = 0;
+ int err;
memset(p, 0, sizeof(*p));
@@ -488,14 +488,13 @@ static int msgctl_stat(struct ipc_namespace *ns, int msqid,
err = PTR_ERR(msq);
goto out_unlock;
}
- success_return = msq->q_perm.id;
+ id = msq->q_perm.id;
} else {
msq = msq_obtain_object_check(ns, msqid);
if (IS_ERR(msq)) {
err = PTR_ERR(msq);
goto out_unlock;
}
- success_return = 0;
}
err = -EACCES;
@@ -506,6 +505,14 @@ static int msgctl_stat(struct ipc_namespace *ns, int msqid,
if (err)
goto out_unlock;
+ ipc_lock_object(&msq->q_perm);
+
+ if (!ipc_valid_object(&msq->q_perm)) {
+ ipc_unlock_object(&msq->q_perm);
+ err = -EIDRM;
+ goto out_unlock;
+ }
+
kernel_to_ipc64_perm(&msq->q_perm, &p->msg_perm);
p->msg_stime = msq->q_stime;
p->msg_rtime = msq->q_rtime;
@@ -515,9 +522,10 @@ static int msgctl_stat(struct ipc_namespace *ns, int msqid,
p->msg_qbytes = msq->q_qbytes;
p->msg_lspid = msq->q_lspid;
p->msg_lrpid = msq->q_lrpid;
- rcu_read_unlock();
- return success_return;
+ ipc_unlock_object(&msq->q_perm);
+ rcu_read_unlock();
+ return id;
out_unlock:
rcu_read_unlock();
diff --git a/ipc/sem.c b/ipc/sem.c
index f7385bce5fd3..9b6f80d1b3f1 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -1211,10 +1211,20 @@ static int semctl_stat(struct ipc_namespace *ns, int semid,
if (err)
goto out_unlock;
+ ipc_lock_object(&sma->sem_perm);
+
+ if (!ipc_valid_object(&sma->sem_perm)) {
+ ipc_unlock_object(&sma->sem_perm);
+ err = -EIDRM;
+ goto out_unlock;
+ }
+
kernel_to_ipc64_perm(&sma->sem_perm, &semid64->sem_perm);
semid64->sem_otime = get_semotime(sma);
semid64->sem_ctime = sma->sem_ctime;
semid64->sem_nsems = sma->sem_nsems;
+
+ ipc_unlock_object(&sma->sem_perm);
rcu_read_unlock();
return id;
diff --git a/ipc/shm.c b/ipc/shm.c
index 565f17925128..8f58faba7429 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -896,9 +896,11 @@ static int shmctl_stat(struct ipc_namespace *ns, int shmid,
int cmd, struct shmid64_ds *tbuf)
{
struct shmid_kernel *shp;
- int result;
+ int id = 0;
int err;
+ memset(tbuf, 0, sizeof(*tbuf));
+
rcu_read_lock();
if (cmd == SHM_STAT) {
shp = shm_obtain_object(ns, shmid);
@@ -906,14 +908,13 @@ static int shmctl_stat(struct ipc_namespace *ns, int shmid,
err = PTR_ERR(shp);
goto out_unlock;
}
- result = shp->shm_perm.id;
+ id = shp->shm_perm.id;
} else {
shp = shm_obtain_object_check(ns, shmid);
if (IS_ERR(shp)) {
err = PTR_ERR(shp);
goto out_unlock;
}
- result = 0;
}
err = -EACCES;
@@ -924,7 +925,14 @@ static int shmctl_stat(struct ipc_namespace *ns, int shmid,
if (err)
goto out_unlock;
- memset(tbuf, 0, sizeof(*tbuf));
+ ipc_lock_object(&shp->shm_perm);
+
+ if (!ipc_valid_object(&shp->shm_perm)) {
+ ipc_unlock_object(&shp->shm_perm);
+ err = -EIDRM;
+ goto out_unlock;
+ }
+
kernel_to_ipc64_perm(&shp->shm_perm, &tbuf->shm_perm);
tbuf->shm_segsz = shp->shm_segsz;
tbuf->shm_atime = shp->shm_atim;
@@ -934,8 +942,9 @@ static int shmctl_stat(struct ipc_namespace *ns, int shmid,
tbuf->shm_lpid = shp->shm_lprid;
tbuf->shm_nattch = shp->shm_nattch;
+ ipc_unlock_object(&shp->shm_perm);
rcu_read_unlock();
- return result;
+ return id;
out_unlock:
rcu_read_unlock();
diff --git a/ipc/util.c b/ipc/util.c
index 78755873cc5b..9736cd66b4cc 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -22,9 +22,12 @@
* tree.
* - perform initial checks (capabilities, auditing and permission,
* etc).
- * - perform read-only operations, such as STAT, INFO commands.
+ * - perform read-only operations, such as INFO command, that
+ * do not demand atomicity
* acquire the ipc lock (kern_ipc_perm.lock) through
* ipc_lock_object()
+ * - perform read-only operations that demand atomicity,
+ * such as STAT command.
* - perform data updates, such as SET, RMID commands and
* mechanism-specific operations (semop/semtimedop,
* msgsnd/msgrcv, shmat/shmdt).
--
2.11.0
Powered by blists - more mailing lists