From patchwork Fri May 9 14:35:09 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Henriques X-Patchwork-Id: 347431 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id B8BFA140078; Sat, 10 May 2014 00:35:27 +1000 (EST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1WiltU-0001RD-Tm; Fri, 09 May 2014 14:35:24 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1WiltJ-0001Pq-Rg for kernel-team@lists.ubuntu.com; Fri, 09 May 2014 14:35:13 +0000 Received: from bl6-54-175.dsl.telepac.pt ([82.155.54.175] helo=localhost) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1WiltJ-00059l-Hn for kernel-team@lists.ubuntu.com; Fri, 09 May 2014 14:35:13 +0000 From: Luis Henriques To: kernel-team@lists.ubuntu.com Subject: [Lucid][CVE-2013-4483] ipc/msg: fix race around refcount Date: Fri, 9 May 2014 15:35:09 +0100 Message-Id: <1399646109-16362-2-git-send-email-luis.henriques@canonical.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1399646109-16362-1-git-send-email-luis.henriques@canonical.com> References: <1399646109-16362-1-git-send-email-luis.henriques@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: kernel-team-bounces@lists.ubuntu.com From: Konstantin Khlebnikov In older kernels (before v3.10) ipc_rcu_hdr->refcount was non-atomic int. There was possuble double-free bug: do_msgsnd() calls ipc_rcu_putref() under msq->q_perm->lock and RCU, while freequeue() calls it while it holds only 'rw_mutex', so there is no sinchronization between them. Two function decrements '2' non-atomically, they both can get '0' as result. do_msgsnd() freequeue() msq = msg_lock_check(ns, msqid); ... ipc_rcu_getref(msq); msg_unlock(msq); schedule(); (caller locks spinlock) expunge_all(msq, -EIDRM); ss_wakeup(&msq->q_senders, 1); msg_rmid(ns, msq); msg_unlock(msq); ipc_lock_by_ptr(&msq->q_perm); ipc_rcu_putref(msq); ipc_rcu_putref(msq); < both may get get --(...)->refcount == 0 > This patch locks ipc_lock and RCU around ipc_rcu_putref in freequeue. ( RCU protects memory for spin_unlock() ) Similar bugs might be in other users of ipc_rcu_putref(). In the mainline this has been fixed in v3.10 indirectly in commmit 6062a8dc0517bce23e3c2f7d2fea5e22411269a3 ("ipc,sem: fine grained locking for semtimedop") by Rik van Riel. That commit optimized locking and converted refcount into atomic. I'm not sure that anybody should care about this bug: it's very-very unlikely and no longer exists in actual mainline. I've found this just by looking into the code, probably this never happens in real life. Signed-off-by: Konstantin Khlebnikov Signed-off-by: Ben Hutchings (cherry picked from 91182754daa6ca26dd2e97ee0b0f6e9e37d33324 3.2.y) CVE-2013-4483 BugLink: http://bugs.launchpad.net/bugs/1248713 Signed-off-by: Luis Henriques Acked-by: Andy Whitcroft --- ipc/msg.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ipc/msg.c b/ipc/msg.c index 779f762..75075fc 100644 --- a/ipc/msg.c +++ b/ipc/msg.c @@ -297,7 +297,9 @@ static void freeque(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp) } atomic_sub(msq->q_cbytes, &ns->msg_bytes); security_msg_queue_free(msq); + ipc_lock_by_ptr(&msq->q_perm); ipc_rcu_putref(msq); + ipc_unlock(&msq->q_perm); } /*