linux-kernel - Re: [PATCH v2 1/2] tpm: Use managed allocation for bios event log

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 for Android: free password hash cracker in your pocket

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

Message-ID: <Y9K42OYk3WX7VH4E@kernel.org>
Date:   Thu, 26 Jan 2023 17:31:04 +0000
From:   Jarkko Sakkinen <jarkko@...nel.org>
To:     Eddie James <eajames@...ux.ibm.com>
Cc:     linux-integrity@...r.kernel.org, linux-kernel@...r.kernel.org,
        jgg@...pe.ca, peterhuewe@....de
Subject: Re: [PATCH v2 1/2] tpm: Use managed allocation for bios event log

On Wed, Jan 25, 2023 at 01:06:31PM -0600, Eddie James wrote:
> 
> On 1/20/23 18:11, Jarkko Sakkinen wrote:
> > On Fri, Jan 13, 2023 at 10:10:16AM -0600, Eddie James wrote:
> > > Since the bios event log is freed in the device release function,
> > > let devres handle the deallocation. This will allow other memory
> > > allocation/mapping functions to be used for the bios event log.
> > > 
> > > Signed-off-by: Eddie James <eajames@...ux.ibm.com>
> > > ---
> > >   drivers/char/tpm/eventlog/acpi.c |  5 +++--
> > >   drivers/char/tpm/eventlog/efi.c  | 13 +++++++------
> > >   drivers/char/tpm/eventlog/of.c   |  3 ++-
> > >   drivers/char/tpm/tpm-chip.c      |  1 -
> > >   4 files changed, 12 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c
> > > index 0913d3eb8d51..40360e599bc3 100644
> > > --- a/drivers/char/tpm/eventlog/acpi.c
> > > +++ b/drivers/char/tpm/eventlog/acpi.c
> > > @@ -14,6 +14,7 @@
> > >    * Access to the event log extended by the TCG BIOS of PC platform
> > >    */
> > > +#include <linux/device.h>
> > >   #include <linux/seq_file.h>
> > >   #include <linux/fs.h>
> > >   #include <linux/security.h>
> > > @@ -135,7 +136,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
> > >   	}
> > >   	/* malloc EventLog space */
> > > -	log->bios_event_log = kmalloc(len, GFP_KERNEL);
> > > +	log->bios_event_log = devm_kmalloc(&chip->dev, len, GFP_KERNEL);
> > >   	if (!log->bios_event_log)
> > >   		return -ENOMEM;
> > > @@ -160,7 +161,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
> > >   	return format;
> > >   err:
> > > -	kfree(log->bios_event_log);
> > > +	devm_kfree(&chip->dev, log->bios_event_log);
> > I wonder do we want to do devm_kfree's at all as the memory is freed during
> > detach, i.e. taken care by devres.
> 
> 
> I think we should since the chip/tpm driver will continue to probe without
> the bios event log. Therefore that memory will be wasted if there is some
> error during bios log setup.

OK, I buy this!

For this patch:

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

If you could refine the description for the 2nd, then these would be ready
to be picked.

> Thanks,
> 
> Eddie

BR, Jarkko