[PATCH v2 0/4] tpm: some tpm small fixes.

[PATCH v2 0/4] tpm: some tpm small fixes.

[Pei Xiao]

This series fixes several small 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.

chhanges in v2:
1.add reviewed-by tag
2.remove patch 5: 
make the tpm_init() error messages consistently prefixed with "tpm: " and
report the actual failure of tpm_dev_common_init().
3.modify patch 1 git commit information.

Pei Xiao (4):
  tpm: tpm_ppi: fix zero-extension of negative error codes
  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

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

-- 
2.25.1

[PATCH v2 1/4] tpm: tpm_ppi: fix zero-extension of negative error codes

[Pei Xiao]

tpm_show_ppi_response() keeps its return value in an acpi_status,
a typedef of u32.  The two's complement of the error code is stored
correctly there, but on return the value is converted to ssize_t and
zero-extended, so the sign is lost: user space receives 0xFFFFFFEA
(4294967274) instead of -EINVAL, which breaks the usual "ret < 0" error
check.
Declare the variable as ssize_t so that negative values survive the
conversion.

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 v2 2/4] 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
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
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 v2 3/4] 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
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
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 v2 4/4] 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
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
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

Re: [PATCH v2 1/4] tpm: tpm_ppi: fix zero-extension of negative error codes

[Jarkko Sakkinen]

On Thu, Oct 08, 2026 at 09:16:20AM +0800, Pei Xiao wrote:
> tpm_show_ppi_response() keeps its return value in an acpi_status,
> a typedef of u32.  The two's complement of the error code is stored
> correctly there, but on return the value is converted to ssize_t and
> zero-extended, so the sign is lost: user space receives 0xFFFFFFEA
> (4294967274) instead of -EINVAL, which breaks the usual "ret < 0" error
> check.
> Declare the variable as ssize_t so that negative values survive the
> conversion.
> 
> 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
> 

OK, cool we went through this so

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Br, Jarkko

Re: [PATCH v2 0/4] tpm: some tpm small fixes.

[Jarkko Sakkinen]

On Thu, Oct 08, 2026 at 09:16:19AM +0800, Pei Xiao wrote:
> This series fixes several small 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.
> 
> chhanges in v2:
> 1.add reviewed-by tag
> 2.remove patch 5: 
> make the tpm_init() error messages consistently prefixed with "tpm: " and
> report the actual failure of tpm_dev_common_init().
> 3.modify patch 1 git commit information.
> 
> Pei Xiao (4):
>   tpm: tpm_ppi: fix zero-extension of negative error codes
>   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
> 
>  drivers/char/tpm/tpm-dev-common.c | 3 +++
>  drivers/char/tpm/tpm_nsc.c        | 8 ++++++--
>  drivers/char/tpm/tpm_ppi.c        | 2 +-
>  3 files changed, 10 insertions(+), 3 deletions(-)
> 
> -- 
> 2.25.1
> 

Right, so I've already applied some of the patches so I'm a bit 
confused what to do with this.

See:

https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/log/

Is there something wrong in the ones that I already applied?

Br, Jarkko

Re: [PATCH v2 0/4] tpm: some tpm small fixes.

[Pei Xiao]

在 2026/10/9 01:51, Jarkko Sakkinen 写道:
> On Thu, Oct 08, 2026 at 09:16:19AM +0800, Pei Xiao wrote:
>> This series fixes several small 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.
>>
>> chhanges in v2:
>> 1.add reviewed-by tag
>> 2.remove patch 5: 
>> make the tpm_init() error messages consistently prefixed with "tpm: " and
>> report the actual failure of tpm_dev_common_init().
>> 3.modify patch 1 git commit information.
>>
>> Pei Xiao (4):
>>   tpm: tpm_ppi: fix zero-extension of negative error codes
>>   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
>>
>>  drivers/char/tpm/tpm-dev-common.c | 3 +++
>>  drivers/char/tpm/tpm_nsc.c        | 8 ++++++--
>>  drivers/char/tpm/tpm_ppi.c        | 2 +-
>>  3 files changed, 10 insertions(+), 3 deletions(-)
>>
>> -- 
>> 2.25.1
>>
> 
> Right, so I've already applied some of the patches so I'm a bit 
> confused what to do with this.
> 
> See:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/log/
> 
> Is there something wrong in the ones that I already applied?
hi Jarkko，
  Because this patch ("tpm: tpm_ppi: fix zero-extension of negative
error codes") did not pass review, I resend a v2 version that includes
the patches that have already passed review and carries their
Reviewed-by tags.

Pei.
thanks!>
> Br, Jarkko