Message ID | 20211128230344.14129-1-heinrich.schuchardt@canonical.com |
---|---|
State | Accepted |
Commit | c3de051c411e024f9e63ab338071584b9460e8b3 |
Delegated to: | Ilias Apalodimas |
Headers | show |
Series | [1/1] tis: fix tpm_tis_remove() | expand |
Hi Heinrich, On Mon, Nov 29, 2021 at 12:03:44AM +0100, Heinrich Schuchardt wrote: > tpm_tis_remove() leads to calling tpm_tis_ready() with the IO region > unmapped and chip->locality == -1 (locality released). This leads to a > crash in mmio_write_bytes(). > > The patch implements these changes: > > tpm_tis_remove(): Unmap the IO region after calling tpm_tis_cleanup(). > > tpm_tis_cleanup(): Request locality before IO output and releasing > locality. > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > --- > drivers/tpm/tpm2_tis_core.c | 6 ++++++ > drivers/tpm/tpm2_tis_mmio.c | 5 ++++- > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/tpm/tpm2_tis_core.c b/drivers/tpm/tpm2_tis_core.c > index ec8c730fe9..51392c4584 100644 > --- a/drivers/tpm/tpm2_tis_core.c > +++ b/drivers/tpm/tpm2_tis_core.c > @@ -378,8 +378,14 @@ out: > int tpm_tis_cleanup(struct udevice *dev) > { > struct tpm_chip *chip = dev_get_priv(dev); > + int ret; > + > + ret = tpm_tis_request_locality(dev, 0); > + if (ret) > + return ret; > > tpm_tis_ready(dev); > + > tpm_tis_release_locality(dev, chip->locality); > > return 0; > diff --git a/drivers/tpm/tpm2_tis_mmio.c b/drivers/tpm/tpm2_tis_mmio.c > index 9cedff2225..a646ce41ff 100644 > --- a/drivers/tpm/tpm2_tis_mmio.c > +++ b/drivers/tpm/tpm2_tis_mmio.c > @@ -118,10 +118,13 @@ iounmap: > static int tpm_tis_remove(struct udevice *dev) > { > struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(dev); > + int ret; > + > + ret = tpm_tis_cleanup(dev); > > iounmap(drv_data->iobase); > > - return tpm_tis_cleanup(dev); > + return ret; > } > > static const struct tpm_ops tpm_tis_ops = { > -- > 2.32.0 > Thanks for looking into this Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
diff --git a/drivers/tpm/tpm2_tis_core.c b/drivers/tpm/tpm2_tis_core.c index ec8c730fe9..51392c4584 100644 --- a/drivers/tpm/tpm2_tis_core.c +++ b/drivers/tpm/tpm2_tis_core.c @@ -378,8 +378,14 @@ out: int tpm_tis_cleanup(struct udevice *dev) { struct tpm_chip *chip = dev_get_priv(dev); + int ret; + + ret = tpm_tis_request_locality(dev, 0); + if (ret) + return ret; tpm_tis_ready(dev); + tpm_tis_release_locality(dev, chip->locality); return 0; diff --git a/drivers/tpm/tpm2_tis_mmio.c b/drivers/tpm/tpm2_tis_mmio.c index 9cedff2225..a646ce41ff 100644 --- a/drivers/tpm/tpm2_tis_mmio.c +++ b/drivers/tpm/tpm2_tis_mmio.c @@ -118,10 +118,13 @@ iounmap: static int tpm_tis_remove(struct udevice *dev) { struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(dev); + int ret; + + ret = tpm_tis_cleanup(dev); iounmap(drv_data->iobase); - return tpm_tis_cleanup(dev); + return ret; } static const struct tpm_ops tpm_tis_ops = {
tpm_tis_remove() leads to calling tpm_tis_ready() with the IO region unmapped and chip->locality == -1 (locality released). This leads to a crash in mmio_write_bytes(). The patch implements these changes: tpm_tis_remove(): Unmap the IO region after calling tpm_tis_cleanup(). tpm_tis_cleanup(): Request locality before IO output and releasing locality. Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> --- drivers/tpm/tpm2_tis_core.c | 6 ++++++ drivers/tpm/tpm2_tis_mmio.c | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-)