lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 24 Nov 2022 14:55:24 +0100
From:   Lino Sanfilippo <LinoSanfilippo@....de>
To:     peterhuewe@....de, jarkko@...nel.org, jgg@...pe.ca
Cc:     stefanb@...ux.vnet.ibm.com, linux@...ewoehner.de,
        linux-integrity@...r.kernel.org, linux-kernel@...r.kernel.org,
        jandryuk@...il.com, pmenzel@...gen.mpg.de, l.sanfilippo@...bus.com,
        LinoSanfilippo@....de, lukas@...ner.de, p.rosenberger@...bus.com
Subject: [PATCH v11 00/14] TPM IRQ fixes

From: Lino Sanfilippo <l.sanfilippo@...bus.com>

This series enables IRQ support for the TPM TIS core. For this reason a
number of bugfixes around the interrupt handling are required (patches 1 to
5).

Patch 6 takes into account that according to the TPM Interface
Specification stsValid and commandRead interrupts might not be supported
by the hardware. For this reason the supported interrupts are first queried
and stored. Then wait_for_tpm_stat() is adjusted to not wait for status
changes that are not reported by interrupts.

Patch 7 moves the interrupt flag checks into an own function as suggested
by Jarkko.

Patch 8 Removes the possibility that tpm_tis_data->locality can be changed
at driver runtime so this variable can be read without the need to protect
it against concurrent modification. 

Patch 9 addresses the issue with concurrent locality handling:
Since the interrupt handler writes the interrupt status registers it needs
to hold the locality. However it runs concurrently to the thread which
triggered the interrupt (e.g. by reading or writing data to the TPM). So
it must take care when claiming and releasing the locality itself,
because it may race with the concurrent running thread which also claims
and releases the locality.
To avoid that both interrupt and concurrent running thread interfere with
each other a locality counter is used which guarantees that at any time
the locality is held as long as it is required by one of both execution
paths.

Patch 10 implements the request of a threaded interrupt handler. This is
needed since SPI uses a mutex for data transmission and since we access the
interrupt status register via SPI in the irq handler we need a sleepable
context.

Patch 11 makes sure that writes to the interrupt register are effective if
done in the interrupt handler.

Patch 12 makes sure that writes to the interrupt and INTERRUPT_VECTOR
and INTERRUPT_ENABLE registers are effective by holding the locality.

Patch 13 makes sure that the TPM is properly initialized after power cycle
before the irq test is done.

Patch 14 enables the test for interrupts by setting the required flag
before the test is executed.

Changes in v11:
- adjust patches 4,5 and 14 slightly to void the consecutive removal and
  re-addition of the "ret" variable in tpm_tis_gen_interrupt()

Changes in v10:
- fix typo in subject line as pointed out by Jason Andryuk
- improve patch "tpm, tpm_tis: Claim locality before writing interrupt
  registers" by extending the scope with held locality". For this reason
  the "Reviewed-by" tag by Jarko and the "Tested-by" tag by Michael have
  been removed.
- add fix to avoid TPM_RC_INITIALIZE after power cycle when testing irqs
  (PATCH 13)
- add fix to restore the old interrupt vector at error (PATCH 4)


Changes in v9:
- add a fix for an issue when interrupts are reenabled on resume (PATCH 11)
- improve the commit message for patch 8 as requested by Jarkko
- improved functions naming
- changed patch 12 (tpm, tpm_tis: Enable interrupt test) to not delete the
  TPM_CHIP_FLAG_IRQ flag any more when tpm2_get_tpm_pt() fails. Due to this
  change the 'Tested-by' tag from Michael and the 'Reviewed-by:' tag from
  Jarko has been removed

Changes in v8:
- tpm_tis_data->locality is not changed at runtime any more so that it can
be read without any protection against concurrent modification.
- add missing brackets as pointed out by Jason Andryuk

Changes in v7:
- moved interrupt flag checks into an own function as suggested by Jarkko
- added "Tested-by" tags for Tests from Michael Niewöhner
- fixed one comment

Changes in v6:
- set TPM_TIS_IRQ_TESTED in flag member of the tpm_tis_data struct instead
in an own bitfield 
- improve commit messages
- use int_mask instead of irqs_in_use as variable name
- use sts_mask instead of active_irqs as variable name
- squash patch 5 and 6
- prefix functions with tpm_tis_
- remove "fixes" tag

Changes in v5:
- improve commit message of patch 1 as requested by Jarko
- drop patch that makes locality handling simpler by only claiming it at
  driver startup and releasing it at driver shutdown (requested by Jarko)
- drop patch that moves the interrupt test from tpm_tis_send()
  to tmp_tis_probe_irq_single() as requested by Jarko
- add patch to make locality handling threadsafe so that it can also be
  done by the irq handler
- separate logical changes into own patches
- always request threaded interrupt handler

Changes in v4:
- only request threaded irq in case of SPI as requested by Jarko.
- reimplement patch 2 to limit locality handling changes to the TIS core.
- separate fixes from cleanups as requested by Jarko.
- rephrase commit messages 

Changes in v3:
- fixed compiler error reported by kernel test robot
- rephrased commit message as suggested by Jarko Sakkinen
- added Reviewed-by tag

Changes in v2:
- rebase against 5.12
- free irq on error path


Lino Sanfilippo (14):
  tpm, tpm_tis: Avoid cache incoherency in test for interrupts
  tpm, tpm_tis: Claim locality before writing TPM_INT_ENABLE register
  tpm, tpm_tis: Disable interrupts if tpm_tis_probe_irq() failed
  tpm, tpm_tis: Do not skip reset of original interrupt vector
  tpm, tpm_tis: Claim locality before writing interrupt registers
  tpm, tpm_tis: Only handle supported interrupts
  tpm, tpm_tis: Move interrupt mask checks into own function
  tpm, tpm_tis: do not check for the active locality in interrupt
    handler
  tpm, tpm: Implement usage counter for locality
  tpm, tpm_tis: Request threaded interrupt handler
  tpm, tpm_tis: Claim locality in interrupt handler
  tpm, tpm_tis: Claim locality when interrupts are reenabled on resume
  tpm, tpm_tis: startup chip before testing for interrupts
  tpm, tpm_tis: Enable interrupt test

 drivers/char/tpm/tpm-chip.c     |  38 ++--
 drivers/char/tpm/tpm.h          |   1 +
 drivers/char/tpm/tpm_tis.c      |   2 +-
 drivers/char/tpm/tpm_tis_core.c | 299 ++++++++++++++++++++------------
 drivers/char/tpm/tpm_tis_core.h |   5 +-
 5 files changed, 214 insertions(+), 131 deletions(-)


base-commit: 30a0b95b1335e12efef89dd78518ed3e4a71a763
-- 
2.36.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ