RE: [PATCH] spi: fsl-dspi: fix NULL pointer dereference

[Qiang Zhao <qiang.zhao@xxxxxxx>]

On Thu, Oct 30, 2020 at 21:18PM +0800, Vladimir Oltean <olteanv@xxxxxxxxx> wrote:

> -----Original Message-----
> From: Vladimir Oltean <olteanv@xxxxxxxxx>
> Sent: 2020年10月30日 21:18
> To: Qiang Zhao <qiang.zhao@xxxxxxx>
> Cc: broonie@xxxxxxxxxx; linux-spi@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] spi: fsl-dspi: fix NULL pointer dereference
> 
> On Thu, Oct 29, 2020 at 04:40:35PM +0800, Qiang Zhao wrote:
> > From: Zhao Qiang <qiang.zhao@xxxxxxx>
> >
> > Since commit 530b5affc675 ("spi: fsl-dspi: fix use-after-free in
> > remove path"), this driver causes a kernel oops:
> >
> > [   64.587431] Unable to handle kernel NULL pointer dereference at
> > virtual address 0000000000000020
> > [..]
> > [   64.756080] Call trace:
> > [   64.758526]  dspi_suspend+0x30/0x78
> > [   64.762012]  platform_pm_suspend+0x28/0x70
> >
> > This is because since this commit, the drivers private data point to
> > "dspi" instead of "ctlr", the codes in suspend and resume func were
> > not modified correspondly.
> >
> > Fixes: 530b5affc675 ("spi: fsl-dspi: fix use-after-free in remove
> > path")
> > Signed-off-by: Zhao Qiang <qiang.zhao@xxxxxxx>
> > ---
> 
> Reviewed-by: Vladimir Oltean <olteanv@xxxxxxxxx>
> 
> Please resend with Mark's comment. I would prefer that you even remove the
> stack trace completely and make it more obvious in the commit message itself
> that the NULL pointer occurs during suspend/resume.
> Somehow that managed to get obscured in your current version. It is also not
> helpful at all that there already exists a commit titled 'spi:
> fsl-dspi: fix NULL pointer dereference' on this driver. This causes confusion for
> backporters. Please provide a unique commit message.
> Thanks.

How about it looks like below:

    spi: fsl-dspi: fix wrong pointer in suspend/resume

    Since commit 530b5affc675 ("spi: fsl-dspi: fix use-after-free in
    remove path"), this driver causes a "NULL pointer dereference"
    in dspi_suspend/resume.
    This is because since this commit, the drivers private data point to
    "dspi" instead of "ctlr", the codes in suspend and resume func were
    not modified correspondly.


Best Regards,
Qiang Zhao