Message ID | 20230818041327.gonna.210-kees@kernel.org (mailing list archive) |
---|---|
State | Mainlined |
Commit | 2ddd3cac1fa988323684cee567356e970e6750bd |
Headers | show |
Series | [v2] nsproxy: Convert nsproxy.count to refcount_t | expand |
On Thu, Aug 17, 2023 at 09:13:32PM -0700, Kees Cook wrote: > From: Elena Reshetova <elena.reshetova@intel.com> > > atomic_t variables are currently used to implement reference counters > with the following properties: > - counter is initialized to 1 using atomic_set() > - a resource is freed upon counter reaching zero > - once counter reaches zero, its further > increments aren't allowed > - counter schema uses basic atomic operations > (set, inc, inc_not_zero, dec_and_test, etc.) > > Such atomic variables should be converted to a newly provided > refcount_t type and API that prevents accidental counter overflows and > underflows. This is important since overflows and underflows can lead > to use-after-free situation and be exploitable. > > The variable nsproxy.count is used as pure reference counter. Convert it > to refcount_t and fix up the operations. > > **Important note for maintainers: > > Some functions from refcount_t API defined in refcount.h have different > memory ordering guarantees than their atomic counterparts. Please check > Documentation/core-api/refcount-vs-atomic.rst for more information. > > Normally the differences should not matter since refcount_t provides > enough guarantees to satisfy the refcounting use cases, but in some > rare cases it might matter. Please double check that you don't have > some undocumented memory guarantees for this variable usage. > > For the nsproxy.count it might make a difference in following places: > - put_nsproxy() and switch_task_namespaces(): decrement in > refcount_dec_and_test() only provides RELEASE ordering and ACQUIRE > ordering on success vs. fully ordered atomic counterpart > > Suggested-by: Kees Cook <keescook@chromium.org> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> > Reviewed-by: David Windsor <dwindsor@gmail.com> > Reviewed-by: Hans Liljestrand <ishkamiel@gmail.com> > Signed-off-by: Kees Cook <keescook@chromium.org> > --- Reviewed-by: Christian Brauner <brauner@kernel.org>
diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h index fee881cded01..771cb0285872 100644 --- a/include/linux/nsproxy.h +++ b/include/linux/nsproxy.h @@ -29,7 +29,7 @@ struct fs_struct; * nsproxy is copied. */ struct nsproxy { - atomic_t count; + refcount_t count; struct uts_namespace *uts_ns; struct ipc_namespace *ipc_ns; struct mnt_namespace *mnt_ns; @@ -102,14 +102,13 @@ int __init nsproxy_cache_init(void); static inline void put_nsproxy(struct nsproxy *ns) { - if (atomic_dec_and_test(&ns->count)) { + if (refcount_dec_and_test(&ns->count)) free_nsproxy(ns); - } } static inline void get_nsproxy(struct nsproxy *ns) { - atomic_inc(&ns->count); + refcount_inc(&ns->count); } #endif diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c index 80d9c6d77a45..15781acaac1c 100644 --- a/kernel/nsproxy.c +++ b/kernel/nsproxy.c @@ -30,7 +30,7 @@ static struct kmem_cache *nsproxy_cachep; struct nsproxy init_nsproxy = { - .count = ATOMIC_INIT(1), + .count = REFCOUNT_INIT(1), .uts_ns = &init_uts_ns, #if defined(CONFIG_POSIX_MQUEUE) || defined(CONFIG_SYSVIPC) .ipc_ns = &init_ipc_ns, @@ -55,7 +55,7 @@ static inline struct nsproxy *create_nsproxy(void) nsproxy = kmem_cache_alloc(nsproxy_cachep, GFP_KERNEL); if (nsproxy) - atomic_set(&nsproxy->count, 1); + refcount_set(&nsproxy->count, 1); return nsproxy; }