>-----Original Message----- >From: Jarkko Sakkinen [mailto:jarkko.sakkinen@xxxxxxxxxxxxxxx] >Sent: Friday, December 22, 2017 10:42 AM >To: Shaikh, Azhar <azhar.shaikh@xxxxxxxxx> >Cc: jgg@xxxxxxxx; javierm@xxxxxxxxxx; peterhuewe@xxxxxx; linux-security- >module@xxxxxxxxxxxxxxx; linux-integrity@xxxxxxxxxxxxxxx; linux- >kernel@xxxxxxxxxxxxxxx >Subject: Re: [PATCH v2] tpm: Fix the driver cleanup code > >On Thu, Dec 21, 2017 at 03:13:39PM -0800, Azhar Shaikh wrote: >> Commit 3c1701339284353c41 ("tpm: Keep CLKRUN enabled throughout the >> duration of transmit_cmd()") added code which accessed >> chip->ops, even after it was set to NULL in tpm_del_char_device(), >> called from tpm_chip_unregister() in error / driver exit paths. >> So fix this code. >> >> Fixes: 3c1701339284353c41 ("tpm: Keep CLKRUN enabled throughout the >> duration of transmit_cmd()") >> >> Suggested-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> >> Suggested-by: Jason Gunthorpe <jgg@xxxxxxxx> >> Signed-off-by: Azhar Shaikh <azhar.shaikh@xxxxxxxxx> >> --- >> Changes in v2: >> - Remove TPM_CHIP_FLAG_DO_NOT_CLEAR_OPS flag, instead call >> tpm_tis_clkrun_enable() directly in tpm_tis_remove() >> - Add comment to tpm_tis_clkrun_enable() >> >> drivers/char/tpm/tpm_tis.c | 6 ------ >> drivers/char/tpm/tpm_tis_core.c | 29 +++++++++++++++++------------ >> 2 files changed, 17 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c >> index d29add49b033..c847fc69a2fc 100644 >> --- a/drivers/char/tpm/tpm_tis.c >> +++ b/drivers/char/tpm/tpm_tis.c >> @@ -275,9 +275,6 @@ static void tpm_tis_pnp_remove(struct pnp_dev >> *dev) >> >> tpm_chip_unregister(chip); >> tpm_tis_remove(chip); >> - if (is_bsw()) >> - iounmap(priv->ilb_base_addr); >> - >> } >> >> static struct pnp_driver tis_pnp_driver = { @@ -329,9 +326,6 @@ >> static int tpm_tis_plat_remove(struct platform_device *pdev) >> tpm_chip_unregister(chip); >> tpm_tis_remove(chip); >> >> - if (is_bsw()) >> - iounmap(priv->ilb_base_addr); >> - >> return 0; >> } >> >> diff --git a/drivers/char/tpm/tpm_tis_core.c >> b/drivers/char/tpm/tpm_tis_core.c index c2227983ed88..519e4a78c8f8 >> 100644 >> --- a/drivers/char/tpm/tpm_tis_core.c >> +++ b/drivers/char/tpm/tpm_tis_core.c >> @@ -37,6 +37,8 @@ >> */ >> #define TPM_POLL_SLEEP 1 /* msec */ >> >> +static void tpm_tis_clkrun_enable(struct tpm_chip *chip, bool value); >> + >> static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask, >> bool check_cancel, bool *canceled) { >@@ -716,8 +718,7 @@ void >> tpm_tis_remove(struct tpm_chip *chip) >> u32 interrupt; >> int rc; >> >> - if (chip->ops->clk_enable != NULL) >> - chip->ops->clk_enable(chip, true); >> + tpm_tis_clkrun_enable(chip, true); >> >> rc = tpm_tis_read32(priv, reg, &interrupt); >> if (rc < 0) >> @@ -725,8 +726,10 @@ void tpm_tis_remove(struct tpm_chip *chip) >> >> tpm_tis_write32(priv, reg, ~TPM_GLOBAL_INT_ENABLE & interrupt); >> >> - if (chip->ops->clk_enable != NULL) >> - chip->ops->clk_enable(chip, false); >> + tpm_tis_clkrun_enable(chip, false); >> + >> + if (priv->ilb_base_addr) >> + iounmap(priv->ilb_base_addr); >> } >> EXPORT_SYMBOL_GPL(tpm_tis_remove); >> >> @@ -736,6 +739,9 @@ void tpm_tis_remove(struct tpm_chip *chip) >> * @chip: TPM chip to use >> * @value: 1 - Disable CLKRUN protocol, so that clocks are free running >> * 0 - Enable CLKRUN protocol >> + * >> + * Call this function directly in tpm_tis_remove() in error or driver >> + removal >> + * path, since the chip->ops is set to NULL in tpm_chip_unregister(). >> */ >> static void tpm_tis_clkrun_enable(struct tpm_chip *chip, bool value) >> { @@ -922,21 +928,20 @@ int tpm_tis_core_init(struct device *dev, >> struct tpm_tis_data *priv, int irq, >> } >> >> rc = tpm_chip_register(chip); >> - if (rc && is_bsw()) >> - iounmap(priv->ilb_base_addr); >> + if (rc) >> + goto out_err; >> >> if (chip->ops->clk_enable != NULL) >> chip->ops->clk_enable(chip, false); >> >> - return rc; >> -out_err: >> - tpm_tis_remove(chip); >> - if (is_bsw()) >> - iounmap(priv->ilb_base_addr); >> + return 0; >> >> - if (chip->ops->clk_enable != NULL) >> +out_err: >> + if ((chip->ops != NULL) && (chip->ops->clk_enable != NULL)) >> chip->ops->clk_enable(chip, false); >> >> + tpm_tis_remove(chip); >> + >> return rc; >> } >> EXPORT_SYMBOL_GPL(tpm_tis_core_init); >> -- >> 1.9.1 >> > >Since the original commit is not in the mainline yet, you should rather >send an updated version of that commit. > Ok, I will send an updated version of the original commit "tpm: Keep CLKRUN enabled throughout the duration of transmit_cmd()" >/Jarkko Regards, Azhar Shaikh
