linux-kernel - Re: [PATCH 8/8] efi: pstore: Add module parameter for setting the record size

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]

Date:   Fri, 7 Oct 2022 14:01:53 -0300
From:   "Guilherme G. Piccoli" <gpiccoli@...lia.com>
To:     Ard Biesheuvel <ardb@...nel.org>
Cc:     Kees Cook <keescook@...omium.org>, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, kernel-dev@...lia.com,
        kernel@...ccoli.net, anton@...msg.org, ccross@...roid.com,
        tony.luck@...el.com, linux-efi@...r.kernel.org
Subject: Re: [PATCH 8/8] efi: pstore: Add module parameter for setting the
 record size

On 07/10/2022 12:06, Ard Biesheuvel wrote:
> [...]
> Well, I did spot this oddity
> 
>         efi_pstore_info.buf = kmalloc(4096, GFP_KERNEL);
>         if (!efi_pstore_info.buf)
>                 return -ENOMEM;
> 
>        efi_pstore_info.bufsize = 1024;
> 
> So that hardcoded 4096 looks odd, but at least it is larger than the
> default 1024. So what happens if you increase the record size to >
> 4096?

This is a very good finding, thanks a bunch Ard and apologies for this
mistake!

Before this patch it was "safe" doing this way since the allocation was
4096 whereas the size value was 1024. Now, with my change this is not
valid anymore, and my feeling is that it worked fine in my tests because
I'm testing panic (which is a single CPU/no-IRQ scenario), so basically
we're corrupting memory...but nothing broke in my tests due to panic
scenario.

Thanks again, I'll fix that - need to allocate record_size.

Guilherme