linux-kernel - Re: [PATCH v5 03/10] kexec_file: factor out arch_kexec_kernel

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:   Wed, 11 Oct 2017 07:55:20 +0100
From:   Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:     AKASHI Takahiro <takahiro.akashi@...aro.org>
Cc:     Julien Thierry <julien.thierry@....com>, catalin.marinas@....com,
        will.deacon@....com, bauerman@...ux.vnet.ibm.com,
        dhowells@...hat.com, vgoyal@...hat.com,
        herbert@...dor.apana.org.au, davem@...emloft.net,
        akpm@...ux-foundation.org, mpe@...erman.id.au, dyoung@...hat.com,
        bhe@...hat.com, arnd@...db.de, kexec@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 03/10] kexec_file: factor out arch_kexec_kernel_*() from x86, powerpc

Hello Takahiro-san,

> On 11 Oct 2017, at 06:07, AKASHI Takahiro <takahiro.akashi@...aro.org> wrote:
> 
> On Tue, Oct 10, 2017 at 12:02:01PM +0100, Julien Thierry wrote:
> 
> [snip]
> 
>>> --- a/kernel/kexec_file.c
>>> +++ b/kernel/kexec_file.c
>>> @@ -26,30 +26,79 @@
>>> #include <linux/vmalloc.h>
>>> #include "kexec_internal.h"
>>> 
>>> +const __weak struct kexec_file_ops * const kexec_file_loaders[] = {NULL};
>>> +
>>> static int kexec_calculate_store_digests(struct kimage *image);
>>> 
>>> +int _kexec_kernel_image_probe(struct kimage *image, void *buf,
>>> +                          unsigned long buf_len)
>>> +{
>>> +     const struct kexec_file_ops *fops;
>>> +     int ret = -ENOEXEC;
>>> +
>>> +     for (fops = kexec_file_loaders[0]; fops && fops->probe; ++fops) {
>> 
>> Hmm, that's not gonna work (and I see that what I said in the previous
>> patch was not 100% correct either).
> 
> Can you elaborate this a bit more?
> 
> I'm sure that, with my code, any member of fops, cannot be changed;
> "const struct kexec_file_ops *fops" means that fops is a pointer to
> "constant sturct kexec_file_ops," while "struct kexec_file_ops *
> const kexec_file_loaders[]" means that kexec_file_loaders is a "constant
> array" of pointers to "constant struct kexec_file_ops."
> 

No, you need 2x const for that, i.e.,

const struct kexec_file_ops * const kexec_file_loaders[]

otherwise, the pointed-to objects may still be modified. 





> Thanks,
> -Takahiro AKASHI
> 
> 
>> 'fops' should be of type 'const struct kexec_file_ops **', and the loop
>> should be:
>> 
>> for (fops = &kexec_file_loaders[0]; *fops && (*fops)->probe; ++fops)
>> 
>> With some additional dereferences in the body of the loop.
>> 
>> Unless you prefer the previous state of the loop (with i and the break
>> inside), but I still think this looks better.
>> 
>> Cheers,
>> 
>> --
>> Julien Thierry
>> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.