RE: [PATCH 1/4] s390: convert debug_info.ref_count from atomic_t to refcount_t

[Reshetova, Elena]

> 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
as before.

Best Regards,
Elena.