Re: [PATCH v2 1/3] char: tpm: Protect tpm_pm_suspend with locks

[Jarkko Sakkinen <jarkko@kernel.org>]

On Mon, Nov 07, 2022 at 09:45:41AM +0100, Jan Dąbroś wrote:
> niedz., 6 lis 2022 o 20:49 Jarkko Sakkinen <jarkko@kernel.org> napisał(a):
> >
> > On Thu, Nov 03, 2022 at 03:54:48PM +0100, Jan Dabros wrote:
> > > Currently tpm transactions are executed unconditionally in
> > > tpm_pm_suspend() function, what may lead to races with other tpm
> > > accessors in the system.
> > >
> > > Add proper locking mechanisms by calling tpm_try_get_ops() which is a
> > > wrapper on tpm_chip_start().
> > >
> > > Signed-off-by: Jan Dabros <jsd@semihalf.com>
> >
> > AFAIK processes are freezed before suspend callbacks are called, and
> > the callbacks are called sequentially. I have no idea what is meant
> > by "TPM accessor" here.
> 
> User space processes are freezed before suspend, but kernel threads
> are not freezable by default. In my particular case it was a hwrng
> thread started from drivers/char/hw_random/core.c - I was referring to
> it as "TPM accessor". For sure I should be more precise in a commit
> msg.

OK, great.

> 
> > Please describe the concurrency scenario in the commit message where the
> > race could happen, if it is hard to reproduce, and add an appropriate fixes
> > tag.
> 
> I will describe my scenario in more detail in the next version.
> Regarding the "fixes" tag - I'm not too familiar with it, but looking
> at the kernel submission guide, "fixes" should be used either when
> there was a particular commit in the past which introduced the bug or
> if a patch fixes an already logged bug entry (so that one can paste
> URL). In my case both are not applicable, so please advise what
> exactly I should put after this tag?

It is 

Fixed: <12 character prefix of the hash> ("short summary")

It should point out to the commit, which introduced the issue/bug.

> Best Regards,
> Jan

BR, Jarkko