linux-kernel - Re: [PATCH v2] tpm: Cleanup class for 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] [day] [month] [year] [list]

Message-ID: <aF9QHdH9k8x1mVjy@kernel.org>
Date: Sat, 28 Jun 2025 05:14:53 +0300
From: Jarkko Sakkinen <jarkko@...nel.org>
To: linux-kernel@...r.kernel.org
Cc: keyrings@...r.kernel.org, Jarkko Sakkinen <jarkko.sakkinen@...nsys.com>,
	Peter Huewe <peterhuewe@....de>, Jason Gunthorpe <jgg@...pe.ca>,
	James Bottomley <James.Bottomley@...senpartnership.com>,
	Mimi Zohar <zohar@...ux.ibm.com>,
	David Howells <dhowells@...hat.com>,
	Paul Moore <paul@...l-moore.com>, James Morris <jmorris@...ei.org>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	"open list:TPM DEVICE DRIVER" <linux-integrity@...r.kernel.org>,
	"open list:SECURITY SUBSYSTEM" <linux-security-module@...r.kernel.org>
Subject: Re: [PATCH v2] tpm: Cleanup class for tpm_buf

On Thu, Jun 26, 2025 at 01:19:33PM +0300, Jarkko Sakkinen wrote:
> From: Jarkko Sakkinen <jarkko.sakkinen@...nsys.com>
> 
> Create a cleanup class for struct tpm_buf using DEFINE_CLASS(), which will
> guarantee that the heap allocated memory will be freed automatically for
> the transient instances of this structure, when they go out of scope.
> 
> Wrap this all into help macro CLASS_TPM_BUF().
> 
> A TPM buffer can now be declared trivially:
> 
>     CLASS_TPM_BUF(buf, buf_size);

Right, so learning this while doing and probably DEFINE_CLASS() would be
a bad idea :-) There's a better fit in cleanup.h: DEFINE_FREE() and
__free().

Given thatintroduction of tpm_buf_alloc() wipes out tpm_buf_destroy(),
we don't need to create any new wrappers with DEFINE_FREE() as
linux/slab.h has kfree() covered already.

This leads up to "one step backwards" solution i.e., explicitly call
tpm_buf_alloc() and implictly destroy (null checks are left out from
examples):

	struct tpm_buf *buf __free(kfree) = tpm_buf_alloc();
	/* 
	 * I dropped buf_size as it will be gone in v3 as requested by
	 * James earlier.
	 */

	 /* Or: */
	struct tpm_buf *buf2 __free(kfree) = NULL;

	/* ... */
	buf2 = tpm_buf_alloc();

OFF-TOPIC: while doing this patch I noticed maybe 3-5 location where
we have do this:

1. Init something that is more complex to rollback than rolling back
   tpm_buf (which is kfree).
2. Init tpm_buf.
3. After this guaranteed success.

Only reason for doing the rollback for the "more complex to rollback
thing" is that stupid placement of tpm_buf_init(). There's no additional
conditionally failing steps after it.

I need to relocate these code blocks and do a reorders as split patches
and place them to the head of the patch set.

This was mostly reminder for myself :-) 

BR, Jarkko