2021-08-09 21:26:58

by Stefan Berger

[permalink] [raw]
Subject: [PATCH v4 0/2] ibmvtpm: Avoid error message when process gets signal while waiting

From: Stefan Berger <[email protected]>

This series of patches fixes an issue related to the ibmvtpm driver causing
unnecessary kernel log messages when a process is interrupted while waiting
for the TPM to respond. The aborted wait causes the core TPM driver to emit
the log message. The solution is to convert the driver to use the normal
polling loop to wait for TPM responses.

Stefan

v4:
- Reverted order of patches

v3:
- Split into two patches

Stefan Berger (2):
tpm: ibmvtpm: Avoid error message when process gets signal while
waiting
tpm: ibmvtpm: Rename tpm_process_cmd to tpm_status and define flag

drivers/char/tpm/tpm_ibmvtpm.c | 31 ++++++++++++++++++-------------
drivers/char/tpm/tpm_ibmvtpm.h | 3 ++-
2 files changed, 20 insertions(+), 14 deletions(-)

--
2.31.1


2021-08-09 21:37:09

by Stefan Berger

[permalink] [raw]
Subject: [PATCH v4 1/2] tpm: ibmvtpm: Avoid error message when process gets signal while waiting

From: Stefan Berger <[email protected]>

When rngd is run as root then lots of these types of message will appear
in the kernel log if the TPM has been configured to provide random bytes:

[ 7406.275163] tpm tpm0: tpm_transmit: tpm_recv: error -4

The issue is caused by the following call that is interrupted while
waiting for the TPM's response.

sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd);

Rather than waiting for the response in the low level driver, have it use
the polling loop in tpm_try_transmit() that uses a command's duration to
poll until a result has been returned by the TPM, thus ending when the
timeout has occurred but not responding to signals and ctrl-c anymore. To
stay in this polling loop extend tpm_ibmvtpm_status() to return
'true' for as long as the vTPM is indicated as being busy in
tpm_processing_cmd. Since the loop requires the TPM's timeouts, get them
now using tpm_get_timeouts() after setting the TPM2 version flag on the
chip.

To recreat the resolved issue start rngd like this:

sudo rngd -r /dev/hwrng -t
sudo rngd -r /dev/tpm0 -t

Link: https://bugzilla.redhat.com/show_bug.cgi?id=1981473
Fixes: 6674ff145eef ("tpm_ibmvtpm: properly handle interrupted packet receptions")
Cc: Nayna Jain <[email protected]>
Cc: George Wilson <[email protected]>
Reported-by: Nageswara R Sastry <[email protected]>
Signed-off-by: Stefan Berger <[email protected]>
---
drivers/char/tpm/tpm_ibmvtpm.c | 20 ++++++++++++--------
drivers/char/tpm/tpm_ibmvtpm.h | 2 +-
2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 903604769de9..7a9eca5768f8 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -106,17 +106,12 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
{
struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
u16 len;
- int sig;

if (!ibmvtpm->rtce_buf) {
dev_err(ibmvtpm->dev, "ibmvtpm device is not ready\n");
return 0;
}

- sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd);
- if (sig)
- return -EINTR;
-
len = ibmvtpm->res_len;

if (count < len) {
@@ -269,7 +264,9 @@ static void tpm_ibmvtpm_cancel(struct tpm_chip *chip)

static u8 tpm_ibmvtpm_status(struct tpm_chip *chip)
{
- return 0;
+ struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
+
+ return ibmvtpm->tpm_processing_cmd;
}

/**
@@ -457,7 +454,7 @@ static const struct tpm_class_ops tpm_ibmvtpm = {
.send = tpm_ibmvtpm_send,
.cancel = tpm_ibmvtpm_cancel,
.status = tpm_ibmvtpm_status,
- .req_complete_mask = 0,
+ .req_complete_mask = true,
.req_complete_val = 0,
.req_canceled = tpm_ibmvtpm_req_canceled,
};
@@ -688,8 +685,15 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
goto init_irq_cleanup;
}

- if (!strcmp(id->compat, "IBM,vtpm20")) {
+
+ if (!strcmp(id->compat, "IBM,vtpm20"))
chip->flags |= TPM_CHIP_FLAG_TPM2;
+
+ rc = tpm_get_timeouts(chip);
+ if (rc)
+ goto init_irq_cleanup;
+
+ if (chip->flags & TPM_CHIP_FLAG_TPM2) {
rc = tpm2_get_cc_attrs_tbl(chip);
if (rc)
goto init_irq_cleanup;
diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
index b92aa7d3e93e..51198b137461 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.h
+++ b/drivers/char/tpm/tpm_ibmvtpm.h
@@ -41,7 +41,7 @@ struct ibmvtpm_dev {
wait_queue_head_t wq;
u16 res_len;
u32 vtpm_version;
- bool tpm_processing_cmd;
+ u8 tpm_processing_cmd;
};

#define CRQ_RES_BUF_SIZE PAGE_SIZE
--
2.31.1

2021-08-10 09:01:17

by R Nageswara Sastry

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] tpm: ibmvtpm: Avoid error message when process gets signal while waiting



On 10/08/21 12:51 am, Stefan Berger wrote:
> From: Stefan Berger <[email protected]>
>
> When rngd is run as root then lots of these types of message will appear
> in the kernel log if the TPM has been configured to provide random bytes:
>
> [ 7406.275163] tpm tpm0: tpm_transmit: tpm_recv: error -4
>
> The issue is caused by the following call that is interrupted while
> waiting for the TPM's response.
>
> sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd);
>
> Rather than waiting for the response in the low level driver, have it use
> the polling loop in tpm_try_transmit() that uses a command's duration to
> poll until a result has been returned by the TPM, thus ending when the
> timeout has occurred but not responding to signals and ctrl-c anymore. To
> stay in this polling loop extend tpm_ibmvtpm_status() to return
> 'true' for as long as the vTPM is indicated as being busy in
> tpm_processing_cmd. Since the loop requires the TPM's timeouts, get them
> now using tpm_get_timeouts() after setting the TPM2 version flag on the
> chip.
>
> To recreat the resolved issue start rngd like this:
>
> sudo rngd -r /dev/hwrng -t
> sudo rngd -r /dev/tpm0 -t
>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1981473
> Fixes: 6674ff145eef ("tpm_ibmvtpm: properly handle interrupted packet receptions")
> Cc: Nayna Jain <[email protected]>
> Cc: George Wilson <[email protected]>
> Reported-by: Nageswara R Sastry <[email protected]>
> Signed-off-by: Stefan Berger <[email protected]>


Tested-by: Nageswara R Sastry <[email protected]>

Tested with /dev/hwrng and /dev/tpm0 and not seen any tpm errors from
the kernel.

> ---
> drivers/char/tpm/tpm_ibmvtpm.c | 20 ++++++++++++--------
> drivers/char/tpm/tpm_ibmvtpm.h | 2 +-
> 2 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
> index 903604769de9..7a9eca5768f8 100644
> --- a/drivers/char/tpm/tpm_ibmvtpm.c
> +++ b/drivers/char/tpm/tpm_ibmvtpm.c
> @@ -106,17 +106,12 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> {
> struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
> u16 len;
> - int sig;
>
> if (!ibmvtpm->rtce_buf) {
> dev_err(ibmvtpm->dev, "ibmvtpm device is not ready\n");
> return 0;
> }
>
> - sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd);
> - if (sig)
> - return -EINTR;
> -
> len = ibmvtpm->res_len;
>
> if (count < len) {
> @@ -269,7 +264,9 @@ static void tpm_ibmvtpm_cancel(struct tpm_chip *chip)
>
> static u8 tpm_ibmvtpm_status(struct tpm_chip *chip)
> {
> - return 0;
> + struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
> +
> + return ibmvtpm->tpm_processing_cmd;
> }
>
> /**
> @@ -457,7 +454,7 @@ static const struct tpm_class_ops tpm_ibmvtpm = {
> .send = tpm_ibmvtpm_send,
> .cancel = tpm_ibmvtpm_cancel,
> .status = tpm_ibmvtpm_status,
> - .req_complete_mask = 0,
> + .req_complete_mask = true,
> .req_complete_val = 0,
> .req_canceled = tpm_ibmvtpm_req_canceled,
> };
> @@ -688,8 +685,15 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
> goto init_irq_cleanup;
> }
>
> - if (!strcmp(id->compat, "IBM,vtpm20")) {
> +
> + if (!strcmp(id->compat, "IBM,vtpm20"))
> chip->flags |= TPM_CHIP_FLAG_TPM2;
> +
> + rc = tpm_get_timeouts(chip);
> + if (rc)
> + goto init_irq_cleanup;
> +
> + if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> rc = tpm2_get_cc_attrs_tbl(chip);
> if (rc)
> goto init_irq_cleanup;
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
> index b92aa7d3e93e..51198b137461 100644
> --- a/drivers/char/tpm/tpm_ibmvtpm.h
> +++ b/drivers/char/tpm/tpm_ibmvtpm.h
> @@ -41,7 +41,7 @@ struct ibmvtpm_dev {
> wait_queue_head_t wq;
> u16 res_len;
> u32 vtpm_version;
> - bool tpm_processing_cmd;
> + u8 tpm_processing_cmd;
> };
>
> #define CRQ_RES_BUF_SIZE PAGE_SIZE
>

--
Thanks and Regards
R.Nageswara Sastry