From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751997AbdJaK6q (ORCPT ); Tue, 31 Oct 2017 06:58:46 -0400 Received: from mx2.suse.de ([195.135.220.15]:48144 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751393AbdJaK6p (ORCPT ); Tue, 31 Oct 2017 06:58:45 -0400 Date: Tue, 31 Oct 2017 11:58:43 +0100 From: Jan Kara To: Elena Reshetova Cc: jack@suse.cz, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, amir73il@gmail.com, peterz@infradead.org, keescook@chromium.org Subject: Re: [PATCH 1/2] fsnotify: convert fsnotify_group.refcnt from atomic_t to refcount_t Message-ID: <20171031105843.GF8989@quack2.suse.cz> References: <1508495162-32339-1-git-send-email-elena.reshetova@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1508495162-32339-1-git-send-email-elena.reshetova@intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri 20-10-17 13:26:01, Elena Reshetova wrote: > 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 fsnotify_group.refcnt is used as pure reference counter. > Convert it to refcount_t and fix up the operations. > > Suggested-by: Kees Cook > Reviewed-by: David Windsor > Reviewed-by: Hans Liljestrand > Signed-off-by: Elena Reshetova Thanks for the patch. I've added it to my tree. Honza > --- > fs/notify/group.c | 6 +++--- > include/linux/fsnotify_backend.h | 3 ++- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/fs/notify/group.c b/fs/notify/group.c > index 3235753..b7a4b6a 100644 > --- a/fs/notify/group.c > +++ b/fs/notify/group.c > @@ -107,7 +107,7 @@ void fsnotify_destroy_group(struct fsnotify_group *group) > */ > void fsnotify_get_group(struct fsnotify_group *group) > { > - atomic_inc(&group->refcnt); > + refcount_inc(&group->refcnt); > } > > /* > @@ -115,7 +115,7 @@ void fsnotify_get_group(struct fsnotify_group *group) > */ > void fsnotify_put_group(struct fsnotify_group *group) > { > - if (atomic_dec_and_test(&group->refcnt)) > + if (refcount_dec_and_test(&group->refcnt)) > fsnotify_final_destroy_group(group); > } > > @@ -131,7 +131,7 @@ struct fsnotify_group *fsnotify_alloc_group(const struct fsnotify_ops *ops) > return ERR_PTR(-ENOMEM); > > /* set to 0 when there a no external references to this group */ > - atomic_set(&group->refcnt, 1); > + refcount_set(&group->refcnt, 1); > atomic_set(&group->num_marks, 0); > atomic_set(&group->user_waits, 0); > > diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h > index c6c6931..20a57ba 100644 > --- a/include/linux/fsnotify_backend.h > +++ b/include/linux/fsnotify_backend.h > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include > > /* > * IN_* from inotfy.h lines up EXACTLY with FS_*, this is so we can easily > @@ -135,7 +136,7 @@ struct fsnotify_group { > * inotify_init() and the refcnt will hit 0 only when that fd has been > * closed. > */ > - atomic_t refcnt; /* things with interest in this group */ > + refcount_t refcnt; /* things with interest in this group */ > > const struct fsnotify_ops *ops; /* how this group handles things */ > > -- > 2.7.4 > -- Jan Kara SUSE Labs, CR