Skip to content

Commit

Permalink
ipc/util.c: use ipc_rcu_putref() for failues in ipc_addid()
Browse files Browse the repository at this point in the history
ipc_addid() is impossible to use:
- for certain failures, the caller must not use ipc_rcu_putref(),
  because the reference counter is not yet initialized.
- for other failures, the caller must use ipc_rcu_putref(),
  because parallel operations could be ongoing already.

The patch cleans that up, by initializing the refcount early, and by
modifying all callers.

The issues is related to the finding of
[email protected]: syzbot found an
issue with reading kern_ipc_perm.seq, here both read and write to already
released memory could happen.

Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Manfred Spraul <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Davidlohr Bueso <[email protected]>
Cc: Davidlohr Bueso <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: Michael Kerrisk <[email protected]>
Cc: Michal Hocko <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
manfred-colorfu authored and torvalds committed Aug 22, 2018
1 parent e2652ae commit 39cfffd
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 4 deletions.
2 changes: 1 addition & 1 deletion ipc/msg.c
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
/* ipc_addid() locks msq upon success. */
retval = ipc_addid(&msg_ids(ns), &msq->q_perm, ns->msg_ctlmni);
if (retval < 0) {
call_rcu(&msq->q_perm.rcu, msg_rcu_free);
ipc_rcu_putref(&msq->q_perm, msg_rcu_free);
return retval;
}

Expand Down
2 changes: 1 addition & 1 deletion ipc/sem.c
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params)
/* ipc_addid() locks sma upon success. */
retval = ipc_addid(&sem_ids(ns), &sma->sem_perm, ns->sc_semmni);
if (retval < 0) {
call_rcu(&sma->sem_perm.rcu, sem_rcu_free);
ipc_rcu_putref(&sma->sem_perm, sem_rcu_free);
return retval;
}
ns->used_sems += nsems;
Expand Down
2 changes: 2 additions & 0 deletions ipc/shm.c
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,8 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
if (is_file_hugepages(file) && shp->mlock_user)
user_shm_unlock(size, shp->mlock_user);
fput(file);
ipc_rcu_putref(&shp->shm_perm, shm_rcu_free);
return error;
no_file:
call_rcu(&shp->shm_perm.rcu, shm_rcu_free);
return error;
Expand Down
10 changes: 8 additions & 2 deletions ipc/util.c
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,9 @@ static inline int ipc_idr_alloc(struct ipc_ids *ids, struct kern_ipc_perm *new)
* Add an entry 'new' to the ipc ids idr. The permissions object is
* initialised and the first free entry is set up and the id assigned
* is returned. The 'new' entry is returned in a locked state on success.
*
* On failure the entry is not locked and a negative err-code is returned.
* The caller must use ipc_rcu_putref() to free the identifier.
*
* Called with writer ipc_ids.rwsem held.
*/
Expand All @@ -261,6 +263,9 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int limit)
kgid_t egid;
int idx, err;

/* 1) Initialize the refcount so that ipc_rcu_putref works */
refcount_set(&new->refcount, 1);

if (limit > IPCMNI)
limit = IPCMNI;

Expand All @@ -269,16 +274,16 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int limit)

idr_preload(GFP_KERNEL);

refcount_set(&new->refcount, 1);
spin_lock_init(&new->lock);
new->deleted = false;
rcu_read_lock();
spin_lock(&new->lock);

current_euid_egid(&euid, &egid);
new->cuid = new->uid = euid;
new->gid = new->cgid = egid;

new->deleted = false;

idx = ipc_idr_alloc(ids, new);
idr_preload_end();

Expand All @@ -291,6 +296,7 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int limit)
}
}
if (idx < 0) {
new->deleted = true;
spin_unlock(&new->lock);
rcu_read_unlock();
return idx;
Expand Down

0 comments on commit 39cfffd

Please sign in to comment.