On Mon, Nov 28, 2022 at 6:04 PM Jason A. Donenfeld <Jason@xxxxxxxxx> wrote:
>
> Hi Jarkko,
>
> On Mon, Nov 07, 2022 at 06:35:25PM +0200, Jarkko Sakkinen wrote:
> > On Mon, Nov 07, 2022 at 09:45:41AM +0100, Jan Dąbroś wrote:
> > > niedz., 6 lis 2022 o 20:49 Jarkko Sakkinen <jarkko@xxxxxxxxxx> 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@xxxxxxxxxxxx>
> > > >
> > > > 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.
>
> Somebody just reported a bug to me in hwrng stuff, which I traced down
> to this lack of tpm_try_get_ops(), and then I coded up the exact same
> patch as Jan's here, saw it fixed the problem, and then realized it was
> already submitted in that exact form.
>
> Because of some recent hwrng changes, this bug will be triggered more
> often than it was before.
>
> So, even though Jan hasn't submitted a v+1, do you think you could take
> this 1/3 commit, fix up the commit message or whatever you need there,
> and get this in for 6.1-rc8? It's an important fix.
>
> Thanks,
> Jason
And if it helps, here's some more metadata:
Tested-by: Jason A. Donenfeld <Jason@xxxxxxxxx>
Reported-by: Vlastimil Babka <vbabka@xxxxxxx>
Link: https://lore.kernel.org/all/c5ba47ef-393f-1fba-30bd-1230d1b4b592@xxxxxxx/
Cc: stable@xxxxxxxxxxxxxxx
