Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752642AbdGJGsb convert rfc822-to-8bit (ORCPT ); Mon, 10 Jul 2017 02:48:31 -0400 Received: from mga07.intel.com ([134.134.136.100]:58741 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750847AbdGJGsa (ORCPT ); Mon, 10 Jul 2017 02:48:30 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,338,1496127600"; d="scan'208";a="1170598824" From: "Reshetova, Elena" To: "Eric W. Biederman" CC: "linux-kernel@vger.kernel.org" , "peterz@infradead.org" , "gregkh@linuxfoundation.org" , "akpm@linux-foundation.org" , "mingo@redhat.com" , "adobriyan@gmail.com" , "serge@hallyn.com" , "arozansk@redhat.com" , "dave@stgolabs.net" , "keescook@chromium.org" , Hans Liljestrand , "David Windsor" Subject: RE: [PATCH 1/3] ipc: convert ipc_namespace.count from atomic_t to refcount_t Thread-Topic: [PATCH 1/3] ipc: convert ipc_namespace.count from atomic_t to refcount_t Thread-Index: AQHS+P6ySQPfUhM6vkqQ9ug58kizeqJMnj2Q Date: Mon, 10 Jul 2017 06:48:22 +0000 Message-ID: <2236FBA76BA1254E88B949DDB74E612B6FF2269B@IRSMSX102.ger.corp.intel.com> References: <1499417992-3238-1-git-send-email-elena.reshetova@intel.com> <1499417992-3238-2-git-send-email-elena.reshetova@intel.com> <87bmottgo4.fsf@xmission.com> In-Reply-To: <87bmottgo4.fsf@xmission.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3235 Lines: 103 > Elena Reshetova writes: > > > refcount_t type and corresponding API should be > > used instead of atomic_t when the variable is used as > > a reference counter. This allows to avoid accidental > > refcounter overflows that might lead to use-after-free > > situations. > > In this patch you can see all of the uses of the count. > What accidental refcount overflows are possible? Even if one can guarantee and prove that in the current implementation there are no overflows possible, we can't say that for sure for any future implementation. Bugs might always happen unfortunately, but if we convert the refcounter to a safer type we can be sure that overflows are not possible. Does it make sense to you? Best Regards, Elena. > > Eric > > > Signed-off-by: Elena Reshetova > > Signed-off-by: Hans Liljestrand > > Signed-off-by: Kees Cook > > Signed-off-by: David Windsor > > --- > > include/linux/ipc_namespace.h | 5 +++-- > > ipc/msgutil.c | 2 +- > > ipc/namespace.c | 4 ++-- > > 3 files changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h > > index 65327ee..e81445c 100644 > > --- a/include/linux/ipc_namespace.h > > +++ b/include/linux/ipc_namespace.h > > @@ -7,6 +7,7 @@ > > #include > > #include > > #include > > +#include > > > > struct user_namespace; > > > > @@ -19,7 +20,7 @@ struct ipc_ids { > > }; > > > > struct ipc_namespace { > > - atomic_t count; > > + refcount_t count; > > struct ipc_ids ids[3]; > > > > int sem_ctls[4]; > > @@ -118,7 +119,7 @@ extern struct ipc_namespace *copy_ipcs(unsigned long > flags, > > static inline struct ipc_namespace *get_ipc_ns(struct ipc_namespace *ns) > > { > > if (ns) > > - atomic_inc(&ns->count); > > + refcount_inc(&ns->count); > > return ns; > > } > > > > diff --git a/ipc/msgutil.c b/ipc/msgutil.c > > index bf74eaa..8459802 100644 > > --- a/ipc/msgutil.c > > +++ b/ipc/msgutil.c > > @@ -29,7 +29,7 @@ DEFINE_SPINLOCK(mq_lock); > > * and not CONFIG_IPC_NS. > > */ > > struct ipc_namespace init_ipc_ns = { > > - .count = ATOMIC_INIT(1), > > + .count = REFCOUNT_INIT(1), > > .user_ns = &init_user_ns, > > .ns.inum = PROC_IPC_INIT_INO, > > #ifdef CONFIG_IPC_NS > > diff --git a/ipc/namespace.c b/ipc/namespace.c > > index b4d80f9..7af6e6b 100644 > > --- a/ipc/namespace.c > > +++ b/ipc/namespace.c > > @@ -50,7 +50,7 @@ static struct ipc_namespace *create_ipc_ns(struct > user_namespace *user_ns, > > goto fail_free; > > ns->ns.ops = &ipcns_operations; > > > > - atomic_set(&ns->count, 1); > > + refcount_set(&ns->count, 1); > > ns->user_ns = get_user_ns(user_ns); > > ns->ucounts = ucounts; > > > > @@ -144,7 +144,7 @@ static void free_ipc_ns(struct ipc_namespace *ns) > > */ > > void put_ipc_ns(struct ipc_namespace *ns) > > { > > - if (atomic_dec_and_lock(&ns->count, &mq_lock)) { > > + if (refcount_dec_and_lock(&ns->count, &mq_lock)) { > > mq_clear_sbinfo(ns); > > spin_unlock(&mq_lock); > > mq_put_mnt(ns);