Re: [PATCH] tpm/eventlog: Use kvmalloc() for event log buffer

[Takashi Iwai <tiwai@suse.de>]

On Fri, 08 Nov 2024 09:48:38 +0100,
Liang, Andy (Linux Ecosystem Engineering) wrote:
> 
> 
> > On Thu, 07 Nov 2024 20:31:37 +0100,
> > Stefan Berger wrote:
> > > 
> > > 
> > > 
> > > On 11/7/24 2:06 PM, Stefan Berger wrote:
> > > > 
> > > > 
> > > > On 11/7/24 7:38 AM, Takashi Iwai wrote:
> > > >> On Thu, 07 Nov 2024 13:17:33 +0100, Paul Menzel wrote:
> >>  >>> 
> >>  >>> Dear Takashi,
> >>  >>> 
> >>  >>> 
> >>  >>> Thank you for the patch.
> >>  >>> 
> >>  >>> Am 07.11.24 um 12:18 schrieb Takashi Iwai:
> >>  >>>> The TPM2 ACPI table may request a large size for the event log, 
> >>  >>>> and it may be over the max size of kmalloc().  When this happens, 
> >>  >>>> the driver
> >>  >>> 
> >>  >>> What is kmalloc()’s maximum size?
> >>  >> 
> >>  >> 128kB or so, IIRC.
> >>  >> And according Andy, the table can be over 4MB.
> >>  > 
> >>  > Can you copy the contents of the file on that machine and tell us 
> >>  > what size it has:
> >>  > 
> >>  > cp /sys/kernel/security/tpm0/binary_bios_measurements ./
> >>  
> >>  Actually, you may need to have the contents parsed by a user space 
> >>  tool since the driver does not detect where the actual end may be:
> >>  
> >>   tsseventextend -if ./binary_bios_measurements -sim -v
> >>  
> >>  This may give you a feeling for how much is in that file and then 
> >>  you'd have to truncate it into half for example and see whether it 
> >>  still parses the same. My point is that we haven't seen such 
> >>  excessive-sized logs so far and following the parsing above we may 
> >>  find something like this more useful than allocating possibly large 
> >>  amounts of memory that a buggy ACPI table indicates (+ notify
> >> manufacturer):
> >>  
> >>    if (len > MAX_TPM_LOG_SIZE) {
> >>        dev_err(&chip->dev, "Truncated excessive-sized TPM log of %d 
> >>  bytes\n", len);
> >>       len = MAX_TPM_LOG_SIZE;
> >>    }
> >>  
> >>  If you send me the log I'd look at it.
> 
> > It's rather a question Andy; could you check give the requested info?
> 
> 
> https://elixir.bootlin.com/linux/v6.8/source/arch/x86/include/asm/page_types.h#L10
> #define PAGE_SHIFT 12
> #define KMALLOC_SHIFT_MAX (MAX_PAGE_ORDER + PAGE_SHIFT)
>  
> https://elixir.bootlin.com/linux/v6.8/source/include/linux/mmzone.h#L30
> #define MAX_PAGE_ORDER 10
>  
> https://elixir.bootlin.com/linux/v6.8/source/include/linux/slab.h#L309
> #define KMALLOC_MAX_SIZE (1UL << KMALLOC_SHIFT_MAX)
> The max size  = (1UL <<  MAX_PAGE_ORDER + PAGE_SHIFT) = ( 1UL << (10 + 12)) = 2^22 =4,194,304 (4MB)
> 
> For the x86, the max size is 4MB. 

Thanks, it was already corrected by Jarkko :)
But what I meant was about the requests:

> cp /sys/kernel/security/tpm0/binary_bios_measurements ./

and

>   tsseventextend -if ./binary_bios_measurements -sim -v

mentioned in the above.  Could you provide the info?


thanks,

Takashi