[PATCH 0/5] tpm: assorted fixes and cleanups

[PATCH 0/5] tpm: assorted fixes and cleanups

[Pei Xiao]

This series fixes several issues found during a code review of
drivers/char/tpm:

  1. tpm_ppi: tpm_show_ppi_response() stores its return value in an
     acpi_status (a u32 typedef), so error codes such as -EINVAL reach
     user space as huge positive values.  Declare the variable as
     ssize_t to match the show callback's return type.

  2. tpm_nsc: tpm_nsc_remove() doubles as the release callback of the
     hand-created platform device and dereferences the chip drvdata
     unconditionally; init failures before tpmm_chip_alloc() crash
     module load.  Return early when the chip has not been created.

  3. tpm_nsc: the cleanup runs twice on module exit because
     tpm_nsc_remove() is both the explicit cleanup and the device
     release callback; the second run operates on an already freed
     chip.  Stop overriding the release callback, which also stops the
     platform object allocation from leaking.

  4. tpm_dev: a zero-length read() discards a pending response,
     breaking the command/response pairing of the TPM character
     devices, although POSIX requires zero-count reads to have no side
     effects.  Return early on a zero count.

  5. tpm-interface: make the tpm_init() error messages consistently
     prefixed with "tpm: " and report the actual failure of
     tpm_dev_common_init().

The series was prepared with AI assistance (GLM-5.3); every change was
reviewed against the code by hand.

Pei Xiao (5):
  tpm: tpm_ppi: fix wrong error code returned to user space
  tpm: tpm_nsc: fix NULL pointer dereference on init failure
  tpm: tpm_nsc: stop using the cleanup callback as dev.release
  tpm: fix zero-length read discarding the pending response
  tpm: fix log messages in tpm_init()

 drivers/char/tpm/tpm-dev-common.c | 3 +++
 drivers/char/tpm/tpm-interface.c  | 6 +++---
 drivers/char/tpm/tpm_nsc.c        | 8 ++++++--
 drivers/char/tpm/tpm_ppi.c        | 2 +-
 4 files changed, 13 insertions(+), 6 deletions(-)

-- 
2.25.1

[PATCH 1/5] tpm: tpm_ppi: fix wrong error code returned to user space

[Pei Xiao]

tpm_show_ppi_response() stores its return value in an acpi_status,
a typedef of u32.  Both error paths of the function (-EINVAL on a
malformed _DSM package, -EFAULT on a non-zero operation return
code) end up as huge positive values when returned as ssize_t, so
user space cannot detect the failure with the usual "ret < 0"
check.

Declare the variable as ssize_t to match the show callback's
return type.

Fixes: 84b1667dea23 ("ACPI / TPM: replace open-coded _DSM code with helper functions")
Assisted-by: GLM-5.3
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/char/tpm/tpm_ppi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_ppi.c b/drivers/char/tpm/tpm_ppi.c
index c9793a3d986d..949fb7055bea 100644
--- a/drivers/char/tpm/tpm_ppi.c
+++ b/drivers/char/tpm/tpm_ppi.c
@@ -234,7 +234,7 @@ static ssize_t tpm_show_ppi_response(struct device *dev,
 				     struct device_attribute *attr,
 				     char *buf)
 {
-	acpi_status status = -EINVAL;
+	ssize_t status = -EINVAL;
 	union acpi_object *obj, *ret_obj;
 	u64 req, res;
 	struct tpm_chip *chip = to_tpm_chip(dev);
-- 
2.25.1

[PATCH 2/5] tpm: tpm_nsc: fix NULL pointer dereference on init failure

[Pei Xiao]

tpm_nsc_remove() is used as the release callback of the hand-created
platform device and dereferences the chip drvdata unconditionally.
If init fails before tpmm_chip_alloc() (e.g. request_region() cannot
claim the ports), the error path drops the last device reference and
the release callback runs with chip == NULL, crashing module init.

Return early when the chip has not been created yet.

Fixes: afb5abc262e9 ("tpm: two-phase chip management functions")
Assisted-by: GLM-5.3
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/char/tpm/tpm_nsc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c
index 879ac88f5783..46a52dc99b14 100644
--- a/drivers/char/tpm/tpm_nsc.c
+++ b/drivers/char/tpm/tpm_nsc.c
@@ -259,7 +259,12 @@ static struct platform_device *pdev = NULL;
 static void tpm_nsc_remove(struct device *dev)
 {
 	struct tpm_chip *chip = dev_get_drvdata(dev);
-	struct tpm_nsc_priv *priv = dev_get_drvdata(&chip->dev);
+	struct tpm_nsc_priv *priv;
+
+	if (!chip)
+		return;
+
+	priv = dev_get_drvdata(&chip->dev);
 
 	tpm_chip_unregister(chip);
 	release_region(priv->base, 2);
-- 
2.25.1

[PATCH 3/5] tpm: tpm_nsc: stop using the cleanup callback as dev.release

[Pei Xiao]

tpm_nsc_remove() is called explicitly from cleanup_nsc() and also
runs as the platform device release callback on the final
platform_device_put(), so the cleanup executes twice on module
exit; the second invocation operates on a chip that has already
been freed by the devm cleanup, and the I/O region is released
twice.

Overwriting the release callback installed by
platform_device_alloc() also keeps platform_device_release() from
running, leaking the platform object allocation.

Leave the default release callback in place; the explicit call in
cleanup_nsc() remains the single cleanup point.

Fixes: 570302a31149 ("[PATCH] tpm: move nsc driver off pci_dev")
Assisted-by: GLM-5.3
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/char/tpm/tpm_nsc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c
index 46a52dc99b14..bcfa2a1a208a 100644
--- a/drivers/char/tpm/tpm_nsc.c
+++ b/drivers/char/tpm/tpm_nsc.c
@@ -327,7 +327,6 @@ static int __init init_nsc(void)
 
 	pdev->num_resources = 0;
 	pdev->dev.driver = &nsc_drv.driver;
-	pdev->dev.release = tpm_nsc_remove;
 
 	if ((rc = platform_device_add(pdev)) < 0)
 		goto err_put_dev;
-- 
2.25.1

[PATCH 4/5] tpm: fix zero-length read discarding the pending response

[Pei Xiao]

POSIX requires that a read() with a count of zero returns zero and
has no other effects.  tpm_common_read() treats such a call as a
consumed response: it marks the pending response as read and drops
it, so the response can never be retrieved; subsequent reads return
zero and the next write() is allowed to overwrite the response
buffer, silently breaking the command/response pairing of the TPM
character devices.

Return early when the caller passes a zero count, leaving any
pending response untouched for the next read.  A zero-length read
will not report a deferred asynchronous error; POSIX permits read()
to skip error detection for a zero count.

Fixes: 9488585b21be ("tpm: add support for partial reads")
Assisted-by: GLM-5.3
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/char/tpm/tpm-dev-common.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index f942c0c8e402..6569212dc6b8 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -134,6 +134,9 @@ ssize_t tpm_common_read(struct file *file, char __user *buf,
 	ssize_t ret_size = 0;
 	int rc;
 
+	if (!size)
+		return 0;
+
 	mutex_lock(&priv->buffer_mutex);
 
 	if (priv->response_length) {
-- 
2.25.1

[PATCH 5/5] tpm: fix log messages in tpm_init()

[Pei Xiao]

Make the error messages consistently prefixed with "tpm: ", and fix
the tpm_dev_common_init() failure message which was wrongly copied
from the previous step.

Assisted-by: GLM-5.3
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/char/tpm/tpm-interface.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1ccdbde98b69..666a1c654c02 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -524,13 +524,13 @@ static int __init tpm_init(void)
 
 	rc = class_register(&tpm_class);
 	if (rc) {
-		pr_err("couldn't create tpm class\n");
+		pr_err("tpm: couldn't create tpm class\n");
 		return rc;
 	}
 
 	rc = class_register(&tpmrm_class);
 	if (rc) {
-		pr_err("couldn't create tpmrm class\n");
+		pr_err("tpm: couldn't create tpmrm class\n");
 		goto out_destroy_tpm_class;
 	}
 
@@ -542,7 +542,7 @@ static int __init tpm_init(void)
 
 	rc = tpm_dev_common_init();
 	if (rc) {
-		pr_err("tpm: failed to allocate char dev region\n");
+		pr_err("tpm: failed to allocate TPM workqueue\n");
 		goto out_unreg_chrdev;
 	}
 
-- 
2.25.1