2019-07-04 07:27:24

by Milan Broz

Subject: [PATCH v2] tpm: Fix null pointer dereference on chip register error path

If clk_enable is not defined and chip initialization
is canceled code hits null dereference.

Easily reproducible with vTPM init fail:
swtpm chardev --tpmstate dir=nonexistent_dir --tpm2 --vtpm-proxy

BUG: kernel NULL pointer dereference, address: 00000000
...
Call Trace:
tpm_chip_start+0x9d/0xa0 [tpm]
tpm_chip_register+0x10/0x1a0 [tpm]
vtpm_proxy_work+0x11/0x30 [tpm_vtpm_proxy]
process_one_work+0x214/0x5a0
worker_thread+0x134/0x3e0
? process_one_work+0x5a0/0x5a0
kthread+0xd4/0x100
? process_one_work+0x5a0/0x5a0
? kthread_park+0x90/0x90
ret_from_fork+0x19/0x24

Fixes: 719b7d81f204 ("tpm: introduce tpm_chip_start() and tpm_chip_stop()")
Cc: [email protected] # v5.1+
Signed-off-by: Milan Broz <[email protected]>
---
drivers/char/tpm/tpm-chip.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 90325e1749fb..db6ac6f83948 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -77,6 +77,18 @@ static int tpm_go_idle(struct tpm_chip *chip)
return chip->ops->go_idle(chip);
}

+static void tpm_clk_enable(struct tpm_chip *chip)
+{
+ if (chip->ops->clk_enable)
+ chip->ops->clk_enable(chip, true);
+}
+
+static void tpm_clk_disable(struct tpm_chip *chip)
+{
+ if (chip->ops->clk_enable)
+ chip->ops->clk_enable(chip, false);
+}
+
/**
* tpm_chip_start() - power on the TPM
* @chip: a TPM chip to use
@@ -89,13 +101,12 @@ int tpm_chip_start(struct tpm_chip *chip)
{
int ret;

- if (chip->ops->clk_enable)
- chip->ops->clk_enable(chip, true);
+ tpm_clk_enable(chip);

if (chip->locality == -1) {
ret = tpm_request_locality(chip);
if (ret) {
- chip->ops->clk_enable(chip, false);
+ tpm_clk_disable(chip);
return ret;
}
}
@@ -103,8 +114,7 @@ int tpm_chip_start(struct tpm_chip *chip)
ret = tpm_cmd_ready(chip);
if (ret) {
tpm_relinquish_locality(chip);
- if (chip->ops->clk_enable)
- chip->ops->clk_enable(chip, false);
+ tpm_clk_disable(chip);
return ret;
}

@@ -124,8 +134,7 @@ void tpm_chip_stop(struct tpm_chip *chip)
{
tpm_go_idle(chip);
tpm_relinquish_locality(chip);
- if (chip->ops->clk_enable)
- chip->ops->clk_enable(chip, false);
+ tpm_clk_disable(chip);
}
EXPORT_SYMBOL_GPL(tpm_chip_stop);

--
2.20.1

2019-07-08 15:27:19

by Jarkko Sakkinen

Subject: Re: [PATCH v2] tpm: Fix null pointer dereference on chip register error path

On Thu, 2019-07-04 at 09:26 +0200, Milan Broz wrote:
> If clk_enable is not defined and chip initialization
> is canceled code hits null dereference.
>
> Easily reproducible with vTPM init fail:
> swtpm chardev --tpmstate dir=nonexistent_dir --tpm2 --vtpm-proxy
>
> BUG: kernel NULL pointer dereference, address: 00000000
> ...
> Call Trace:
> tpm_chip_start+0x9d/0xa0 [tpm]
> tpm_chip_register+0x10/0x1a0 [tpm]
> vtpm_proxy_work+0x11/0x30 [tpm_vtpm_proxy]
> process_one_work+0x214/0x5a0
> worker_thread+0x134/0x3e0
> ? process_one_work+0x5a0/0x5a0
> kthread+0xd4/0x100
> ? process_one_work+0x5a0/0x5a0
> ? kthread_park+0x90/0x90
> ret_from_fork+0x19/0x24
>
> Fixes: 719b7d81f204 ("tpm: introduce tpm_chip_start() and tpm_chip_stop()")
> Cc: [email protected] # v5.1+
> Signed-off-by: Milan Broz <[email protected]>

Looks legit.

Reviewed-by: Jarkko Sakkinen <[email protected]>

/Jarkko

2019-07-08 15:28:16

by Jarkko Sakkinen

Subject: Re: [PATCH v2] tpm: Fix null pointer dereference on chip register error path

On Mon, 2019-07-08 at 17:34 +0300, Jarkko Sakkinen wrote:
> On Thu, 2019-07-04 at 09:26 +0200, Milan Broz wrote:
> > If clk_enable is not defined and chip initialization
> > is canceled code hits null dereference.
> >
> > Easily reproducible with vTPM init fail:
> > swtpm chardev --tpmstate dir=nonexistent_dir --tpm2 --vtpm-proxy
> >
> > BUG: kernel NULL pointer dereference, address: 00000000
> > ...
> > Call Trace:
> > tpm_chip_start+0x9d/0xa0 [tpm]
> > tpm_chip_register+0x10/0x1a0 [tpm]
> > vtpm_proxy_work+0x11/0x30 [tpm_vtpm_proxy]
> > process_one_work+0x214/0x5a0
> > worker_thread+0x134/0x3e0
> > ? process_one_work+0x5a0/0x5a0
> > kthread+0xd4/0x100
> > ? process_one_work+0x5a0/0x5a0
> > ? kthread_park+0x90/0x90
> > ret_from_fork+0x19/0x24
> >
> > Fixes: 719b7d81f204 ("tpm: introduce tpm_chip_start() and tpm_chip_stop()")
> > Cc: [email protected] # v5.1+
> > Signed-off-by: Milan Broz <[email protected]>
>
> Looks legit.
>
> Reviewed-by: Jarkko Sakkinen <[email protected]>

Please check master and next branches from

git://git.infradead.org/users/jjs/linux-tpmdd.git

/Jarkko