linux-kernel - Re: [PATCH v3 4/6] tpm: Support TPM2 sized buffers (TPM2B)

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: <d4157726d924a3ddad477923d6bcb4a8e6a55e60.camel@HansenPartnership.com>
Date:   Fri, 27 Oct 2023 08:32:42 -0400
From:   James Bottomley <James.Bottomley@...senPartnership.com>
To:     Jarkko Sakkinen <jarkko@...nel.org>,
        linux-integrity@...r.kernel.org
Cc:     keyrings@...r.kernel.org,
        William Roberts <bill.c.roberts@...il.com>,
        Stefan Berger <stefanb@...ux.ibm.com>,
        David Howells <dhowells@...hat.com>,
        Jason Gunthorpe <jgg@...pe.ca>,
        Mimi Zohar <zohar@...ux.ibm.com>,
        Peter Huewe <peterhuewe@....de>,
        Paul Moore <paul@...l-moore.com>,
        James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        Jerry Snitselaar <jsnitsel@...hat.com>,
        Mario Limonciello <mario.limonciello@....com>,
        Julien Gomes <julien@...sta.com>,
        open list <linux-kernel@...r.kernel.org>,
        "open list:SECURITY SUBSYSTEM" 
        <linux-security-module@...r.kernel.org>
Subject: Re: [PATCH v3 4/6] tpm: Support TPM2 sized buffers (TPM2B)

On Tue, 2023-10-24 at 04:15 +0300, Jarkko Sakkinen wrote:
> +++ b/drivers/char/tpm/tpm-buf.c
> @@ -7,22 +7,32 @@
>  #include <linux/tpm.h>
>  
>  /**
> - * tpm_buf_init() - Initialize from the heap
> + * tpm_buf_init() - Initialize a TPM buffer
>   * @buf:       A @tpm_buf
> + * @sized:     Represent a sized buffer (TPM2B)
> + * @alloc:     Allocate from the heap
>   *
>   * Initialize all structure fields to zero, allocate a page from the
> heap, and
>   * zero the bytes that the buffer headers will consume.
>   *
>   * Return: 0 or -ENOMEM
>   */
> -int tpm_buf_init(struct tpm_buf *buf)
> +int tpm_buf_init(struct tpm_buf *buf, bool alloc, bool sized)

I think it creates a phenomenally confusing interface to use multiple
booleans because, unlike flags, it's not self describing at point of
use.  The confusion is enormously heightened here by having the doc
book arguments be the reverse of the actual function prototype (I just
tripped over this).

The alloc flag is particularly counter intuitive: if you pass in an
allocated buffer, you expect to be responsible for freeing it again,
but that's not how you use it; you really use it like a reset not an
alloc, which looks odd because you already created a separate
tpm_buf_reset function which can't be used in this case.

Why not replace the alloc flags with two reset functions: one for TPM2B
buffers and one for command buffers?

James