Re: [PATCH] EDAC/ghes: Do not warn when incrementing refcount on 0

[John Garry]

On 22/11/2019 09:01, Borislav Petkov wrote:
> On Thu, Nov 21, 2019 at 09:36:57PM +0000, Robert Richter wrote:
> > Following warning from the refcount framework is seen during ghes
> > initialization:
> >
> >   EDAC MC0: Giving out device to module ghes_edac.c controller ghes_edac: DEV ghes (INTERRUPT)
> >   ------------[ cut here ]------------
> >   refcount_t: increment on 0; use-after-free.
> >   WARNING: CPU: 36 PID: 1 at lib/refcount.c:156 refcount_inc_checked+0x44/0x50
> > [...]
> >   Call trace:
> >    refcount_inc_checked+0x44/0x50
> >    ghes_edac_register+0x258/0x388
> >    ghes_probe+0x28c/0x5f0
> >
> > It warns if the refcount is incremented from zero. This warning is
> > reasonable as a kernel object is typically created with a refcount of
> > one and freed once the refcount is zero. Afterwards the object would
> > be "used-after-free".
> >
> > For ghes the refcount is initialized with zero, and that is why this
> > message is seen when initializing the first instance. However,
> > whenever the refcount is zero, the device will be allocated and
> > registered. Since the ghes_reg_mutex protects the refcount and
> > serializes allocation and freeing of ghes devices, a use-after-free
> > cannot happen here.
> >
> > Instead of using refcount_inc() for the first instance, use
> > refcount_set(). This can be used here because the refcount is zero at
> > this point and can not change due to its protection by the mutex.
> >
> > Reported-by: John Garry <john.garry@xxxxxxxxxx>
> > Tested-by: John Garry <john.garry@xxxxxxxxxx>

According to kernel dev process Doc, this should be explicitly granted, so:
Tested-by: John Garry <john.garry@xxxxxxxxxx>

Thanks,
John

> > Signed-off-by: Robert Richter <rrichter@xxxxxxxxxxx>
> > ---
> >   drivers/edac/ghes_edac.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
>
> Queued, thanks.