Re: [Syzkaller & bisect] There is WARNING in tpm_chip_unregister in upstream patch "tpm: st33zp24: Mark ACPI and OF related data as maybe unused"

[Pengfei Xu]

Hi Jarkko and Krzysztof Kozlowski,

On 2023-04-26 at 21:03:19 +0300, Jarkko Sakkinen wrote:
> On Wed, 2023-04-26 at 15:12 +0200, Krzysztof Kozlowski wrote:
> > On 26/04/2023 11:10, Pengfei Xu wrote:
> > > Hi Jarkko and Krzysztof Kozlowski,
> > > 
> > > Greeting!
> > > 
> > > Platform: x86 platforms
> > > 
> > > There is WARNING in tpm_chip_unregister in upstream patch "tpm: st33zp24: Mark
> > > ACPI and OF related data as maybe unused":
> > > https://lore.kernel.org/lkml/20230424144130.1084795-1-jarkko@xxxxxxxxxx/
> > > -> https://lore.kernel.org/lkml/20230319141354.22907-1-krzysztof.kozlowski@xxxxxxxxxx/
> > > 
> > > We tested Intel internal kernel and found that, the above patch caused below
> > > WARNING and then kernel BUG dmesg info. After reverted above commit on top
> > > of Intel internal kernel, this issue was gone.
> > > I checked that internal commit:"c3985d8b9c22 tpm: st33zp24: Mark ACPI and OF
> > > related data as maybe unused" was same as above link patch.
> > > This issue could be reproduced in 155s in VM.
> > > 
> > > All detailed info: https://github.com/xupengfe/syzkaller_logs/tree/main/230426_132902_tpm_chip_unregister_warning
> > > Syzkaller reproduced code: https://github.com/xupengfe/syzkaller_logs/blob/main/230426_132902_tpm_chip_unregister_warning/repro.c
> > > Syzkaller syscall reproduced steps: https://github.com/xupengfe/syzkaller_logs/blob/main/230426_132902_tpm_chip_unregister_warning/repro.prog
> > > Syzkaller analysis report: https://github.com/xupengfe/syzkaller_logs/blob/main/230426_132902_tpm_chip_unregister_warning/repro.report
> > > Kconfig: https://github.com/xupengfe/syzkaller_logs/blob/main/230426_132902_tpm_chip_unregister_warning/kconfig_origin
> > > Bisect info: https://github.com/xupengfe/syzkaller_logs/blob/main/230426_132902_tpm_chip_unregister_warning/bisect_info.log
> > > 
> > > "
> > > [   24.638052] memfd_create() without MFD_EXEC nor MFD_NOEXEC_SEAL, pid=329 'systemd'
> > > [   28.731375] cgroup: Unknown subsys name 'net'
> > > [   28.741433] cgroup: Unknown subsys name 'rlimit'
> > > [   35.900833] tpm tpm0: Operation Canceled
> > > [   35.901377] ------------[ cut here ]------------
> > > [   35.901648] refcount_t: addition on 0; use-after-free.
> > > [   35.901986] WARNING: CPU: 0 PID: 4095 at lib/refcount.c:25 refcount_warn_saturate+0xe6/0x1c0
> > 
> > Thanks for the report. I am quite surprised that this commit was pointed
> > as possible cause. The patch does not touch anything related to reported
> > issue... At least I do not see it. Could be some tooling problem in your
> > reproduction steps?
   After checking the bisect detailed info, this issue should be related to
   link https://lore.kernel.org/lkml/ZEibZgi8hjfVIBTE@xxxxxxxxxxxxxxxx/

   And this email's bisect info commit was wrong.
   Because https://github.com/xupengfe/syzkaller_logs/blob/main/230426_132902_tpm_chip_unregister_warning/548eb516ec0f7a484a23a902835899341164b8ea_dmesg.log
   Tested problem commit 548eb516ec which should trigger this issue, but it
   met stuck issue as follow and didn't find the problem dmesg info.
"
[   36.521619] tpm tpm2: Operation Canceled
[   36.536390] tpm tpm0: Operation Canceled
[   36.550622] tpm tpm1: Operation Canceled
[   36.597232] tpm tpm3: Operation Canceled
[   36.631157] tpm tpm4: Operation Canceled
[   36.634093] tpm tpm5: Operation Canceled
[   36.639222] tpm tpm6: Operation Canceled
[   36.648086] tpm tpm5: Operation Canceled
[   36.652523] tpm tpm6: Operation Canceled
[   36.660375] tpm tpm5: Operation Canceled
"
  And then the bisect was wrong.

  When do double check step, reverted the commit on top of the kernel, it met
  the same stuck issue accidently again and gave the wrong conclusion:
  https://github.com/xupengfe/syzkaller_logs/blob/main/230426_132902_tpm_chip_unregister_warning/591f7c2026cba9889839a16a1a3579c38ad6234f_c3985d8b9c224b359851f0a521ad25a83db6bdca_revert_dmesg.log
"
[   36.813933] tpm tpm0: Operation Canceled
[   36.819376] tpm tpm1: Operation Canceled
[   36.827303] tpm tpm2: Operation Canceled
[   36.832653] tpm tpm2: Operation Canceled
[   36.851290] tpm tpm4: Operation Canceled
[   36.852480] tpm tpm5: Operation Canceled
[   36.868413] tpm tpm5: Operation Canceled
[   36.912134] tpm tpm6: Operation Canceled
[   36.920819] tpm tpm6: Operation Canceled
"
  So the issue was not related to commit: "tpm: st33zp24: Mark ACPI and OF
  related data as maybe unused", sorry for inconvience!
  And the issue was fixed by Jarkko and here is the link:
  https://lore.kernel.org/lkml/20230426185833.289868-1-jarkko@xxxxxxxxxx/

> 
> I see a critical bug in this commit.
> 
> Because only tpm_tis_core calls tpm_chip_startup(), in effect none of this
> code is ever executd for e.g. tpm_crb and tpm_vtpm_proxy and some other
> drivers that are not based on TIS/FIFO specification.
> 
  Thanks Jarkko's fixed patch!

  Thanks!
  BR.
  -Pengfei

> Lino, can you submit a fix re-enabling non-tpm_tis drivers?
> 
> BR, Jarkko