Re: [PATCH 4/9] tools: convert dso.refcnt from atomic_t to refcount_t

From: Arnaldo Carvalho de Melo
Date: Wed Feb 22 2017 - 15:40:51 EST


Em Wed, Feb 22, 2017 at 05:37:46PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Feb 21, 2017 at 05:34:58PM +0200, Elena Reshetova escreveu:
> > 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.
>
> Fixed by moving the include refcnt.h to later in the includes:

Nope, in this case this trick didn't work, I'm going back and adding
#include <linux/kernel.h> to refcnt.h.

> In file included from /home/acme/git/linux/tools/perf/util/dso.h:4:0,
> from /home/acme/git/linux/tools/perf/util/machine.h:7,
> from tests/thread-mg-share.c:2:
> /home/acme/git/linux/tools/include/linux/refcount.h: In function ârefcount_inc_not_zeroâ:
> /home/acme/git/linux/tools/include/linux/refcount.h:95:23: error: âUINT_MAXâ undeclared (first use in this function)
> REFCOUNT_WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
> ^
> /home/acme/git/linux/tools/include/linux/refcount.h:47:41: note: in definition of macro âREFCOUNT_WARNâ
> #define REFCOUNT_WARN(cond, str) (void)(cond)
> ^~~~
> /home/acme/git/linux/tools/include/linux/refcount.h:95:23: note: each undeclared identifier is reported only once for each function it appears in
> REFCOUNT_WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
> ^
> /home/acme/git/linux/tools/include/linux/refcount.h:47:41: note: in definition of macro âREFCOUNT_WARNâ
> #define REFCOUNT_WARN(cond, str) (void)(cond)
>
>
> > Signed-off-by: Elena Reshetova <elena.reshetova@xxxxxxxxx>
> > Signed-off-by: Hans Liljestrand <ishkamiel@xxxxxxxxx>
> > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> > Signed-off-by: David Windsor <dwindsor@xxxxxxxxx>
> > ---
> > tools/perf/util/dso.c | 6 +++---
> > tools/perf/util/dso.h | 4 ++--
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> > index 3abe337..f88aa44 100644
> > --- a/tools/perf/util/dso.c
> > +++ b/tools/perf/util/dso.c
> > @@ -1109,7 +1109,7 @@ struct dso *dso__new(const char *name)
> > INIT_LIST_HEAD(&dso->node);
> > INIT_LIST_HEAD(&dso->data.open_entry);
> > pthread_mutex_init(&dso->lock, NULL);
> > - atomic_set(&dso->refcnt, 1);
> > + refcount_set(&dso->refcnt, 1);
> > }
> >
> > return dso;
> > @@ -1147,13 +1147,13 @@ void dso__delete(struct dso *dso)
> > struct dso *dso__get(struct dso *dso)
> > {
> > if (dso)
> > - atomic_inc(&dso->refcnt);
> > + refcount_inc(&dso->refcnt);
> > return dso;
> > }
> >
> > void dso__put(struct dso *dso)
> > {
> > - if (dso && atomic_dec_and_test(&dso->refcnt))
> > + if (dso && refcount_dec_and_test(&dso->refcnt))
> > dso__delete(dso);
> > }
> >
> > diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
> > index ecc4bbd..12350b1 100644
> > --- a/tools/perf/util/dso.h
> > +++ b/tools/perf/util/dso.h
> > @@ -1,7 +1,7 @@
> > #ifndef __PERF_DSO
> > #define __PERF_DSO
> >
> > -#include <linux/atomic.h>
> > +#include <linux/refcount.h>
> > #include <linux/types.h>
> > #include <linux/rbtree.h>
> > #include <sys/types.h>
> > @@ -187,7 +187,7 @@ struct dso {
> > void *priv;
> > u64 db_id;
> > };
> > - atomic_t refcnt;
> > + refcount_t refcnt;
> > char name[0];
> > };
> >
> > --
> > 2.7.4