linux-kernel - Re: [PATCH] tpm: Wrap the buffer from the caller to tpm_buf in tpm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190916210454.mq3g2m6s5a2syaxp@cantor>
Date:   Mon, 16 Sep 2019 14:04:54 -0700
From:   Jerry Snitselaar <jsnitsel@...hat.com>
To:     Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        linux-integrity@...r.kernel.org, Mimi Zohar <zohar@...ux.ibm.com>,
        stable@...r.kernel.org, Peter Huewe <peterhuewe@....de>,
        Jason Gunthorpe <jgg@...pe.ca>, Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tpm: Wrap the buffer from the caller to tpm_buf in
 tpm_send()

On Mon Sep 16 19, Jerry Snitselaar wrote:
>On Mon Sep 16 19, Jarkko Sakkinen wrote:
>>tpm_send() does not give anymore the result back to the caller. This
>>would require another memcpy(), which kind of tells that the whole
>>approach is somewhat broken. Instead, as Mimi suggested, this commit
>>just wraps the data to the tpm_buf, and thus the result will not go to
>>the garbage.
>>
>>Obviously this assumes from the caller that it passes large enough
>>buffer, which makes the whole API somewhat broken because it could be
>>different size than @buflen but since trusted keys is the only module
>>using this API right now I think that this fix is sufficient for the
>>moment.
>>
>>In the near future the plan is to replace the parameters with a tpm_buf
>>created by the caller.
>>
>>Reported-by: Mimi Zohar <zohar@...ux.ibm.com>
>>Suggested-by: Mimi Zohar <zohar@...ux.ibm.com>
>>Cc: stable@...r.kernel.org
>>Fixes: 412eb585587a ("use tpm_buf in tpm_transmit_cmd() as the IO parameter")
>>Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
>>---
>>drivers/char/tpm/tpm-interface.c | 8 ++------
>>1 file changed, 2 insertions(+), 6 deletions(-)
>>
>>diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>>index d9ace5480665..2459d36dd8cc 100644
>>--- a/drivers/char/tpm/tpm-interface.c
>>+++ b/drivers/char/tpm/tpm-interface.c
>>@@ -358,13 +358,9 @@ int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen)
>>	if (!chip)
>>		return -ENODEV;
>>
>>-	rc = tpm_buf_init(&buf, 0, 0);
>>-	if (rc)
>>-		goto out;
>>-
>>-	memcpy(buf.data, cmd, buflen);
>>+	buf.data = cmd;
>>	rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to a send a command");
>>-	tpm_buf_destroy(&buf);
>>+
>>out:
>>	tpm_put_ops(chip);
>>	return rc;
>>-- 
>>2.20.1
>>
>
>Nothing uses the out label any longer so it should be dropped as well, but other than that...
>
>Acked-by: Jerry Snitselaar <jsnitsel@...hat.com>

sigh (wrong emacs macro hit), that should be:

Reviewed-by: Jerry Snitselaar <jsnitsel@...hat.com>