RE: [PATCH 1/4] s390: convert debug_info.ref_count from atomic_t to refcount_t
From: Reshetova, Elena
Date: Mon Feb 20 2017 - 08:36:04 EST
> On Mon, Feb 20, 2017 at 01:06:18PM +0200, Elena Reshetova wrote:
> > 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.
> > 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>
> > ---
> > arch/s390/include/asm/debug.h | 3 ++-
> > arch/s390/kernel/debug.c | 8 ++++----
> > 2 files changed, 6 insertions(+), 5 deletions(-)
> I can only see a pull request from Ingo a couple of hours ago for Peter's
> refcount code. So the refcount code is not merged yet. It would have been
> good if you would have waited until it is really merged to avoid confusion.
Sorry, I guess I was a bit too rushy, but I also want to be able to fix all things that come up as I post these before next merge window closes.
> > @@ -361,7 +361,7 @@ debug_info_create(const char *name, int
> pages_per_area, int nr_areas,
> > debug_area_last = rc;
> > rc->next = NULL;
> > - debug_info_get(rc);
> > + refcount_set(&rc->ref_count, 1);
> This is not wrong, but I will remove this hunk before applying your patch,
> since this doesn't look like an obvious correct change at first glance.
It isn't obvious, but needed unfortunately. refcount_inc is done in the way that it won't increment on zero value.
And since for this variable you set the initial refcounter value to zero and then call debug_info_get (that does inc), this
would only WARN and not increment. So for this initial case, we changed it to call refcount_set to "1" to make sure things work