Skip to content

Commit b9a5322

Browse files
committed
Initialize msg/shm IPC objects before doing ipc_addid()
As reported by Dmitry Vyukov, we really shouldn't do ipc_addid() before having initialized the IPC object state. Yes, we initialize the IPC object in a locked state, but with all the lockless RCU lookup work, that IPC object lock no longer means that the state cannot be seen. We already did this for the IPC semaphore code (see commit e8577d1: "ipc/sem.c: fully initialize sem_array before making it visible") but we clearly forgot about msg and shm. Reported-by: Dmitry Vyukov <dvyukov@google.com> Cc: Manfred Spraul <manfred@colorfullife.com> Cc: Davidlohr Bueso <dbueso@suse.de> Cc: stable@vger.kernel.org Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
1 parent 3225031 commit b9a5322

File tree

3 files changed

+18
-17
lines changed

3 files changed

+18
-17
lines changed

Diff for: ipc/msg.c

+7-7
Original file line numberDiff line numberDiff line change
@@ -137,13 +137,6 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
137137
return retval;
138138
}
139139

140-
/* ipc_addid() locks msq upon success. */
141-
id = ipc_addid(&msg_ids(ns), &msq->q_perm, ns->msg_ctlmni);
142-
if (id < 0) {
143-
ipc_rcu_putref(msq, msg_rcu_free);
144-
return id;
145-
}
146-
147140
msq->q_stime = msq->q_rtime = 0;
148141
msq->q_ctime = get_seconds();
149142
msq->q_cbytes = msq->q_qnum = 0;
@@ -153,6 +146,13 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
153146
INIT_LIST_HEAD(&msq->q_receivers);
154147
INIT_LIST_HEAD(&msq->q_senders);
155148

149+
/* ipc_addid() locks msq upon success. */
150+
id = ipc_addid(&msg_ids(ns), &msq->q_perm, ns->msg_ctlmni);
151+
if (id < 0) {
152+
ipc_rcu_putref(msq, msg_rcu_free);
153+
return id;
154+
}
155+
156156
ipc_unlock_object(&msq->q_perm);
157157
rcu_read_unlock();
158158

Diff for: ipc/shm.c

+7-6
Original file line numberDiff line numberDiff line change
@@ -551,12 +551,6 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
551551
if (IS_ERR(file))
552552
goto no_file;
553553

554-
id = ipc_addid(&shm_ids(ns), &shp->shm_perm, ns->shm_ctlmni);
555-
if (id < 0) {
556-
error = id;
557-
goto no_id;
558-
}
559-
560554
shp->shm_cprid = task_tgid_vnr(current);
561555
shp->shm_lprid = 0;
562556
shp->shm_atim = shp->shm_dtim = 0;
@@ -565,6 +559,13 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
565559
shp->shm_nattch = 0;
566560
shp->shm_file = file;
567561
shp->shm_creator = current;
562+
563+
id = ipc_addid(&shm_ids(ns), &shp->shm_perm, ns->shm_ctlmni);
564+
if (id < 0) {
565+
error = id;
566+
goto no_id;
567+
}
568+
568569
list_add(&shp->shm_clist, &current->sysvshm.shm_clist);
569570

570571
/*

Diff for: ipc/util.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,10 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int size)
237237
rcu_read_lock();
238238
spin_lock(&new->lock);
239239

240+
current_euid_egid(&euid, &egid);
241+
new->cuid = new->uid = euid;
242+
new->gid = new->cgid = egid;
243+
240244
id = idr_alloc(&ids->ipcs_idr, new,
241245
(next_id < 0) ? 0 : ipcid_to_idx(next_id), 0,
242246
GFP_NOWAIT);
@@ -249,10 +253,6 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int size)
249253

250254
ids->in_use++;
251255

252-
current_euid_egid(&euid, &egid);
253-
new->cuid = new->uid = euid;
254-
new->gid = new->cgid = egid;
255-
256256
if (next_id < 0) {
257257
new->seq = ids->seq++;
258258
if (ids->seq > IPCID_SEQ_MAX)

0 commit comments

Comments
 (0)