linux-kernel - Re: [PATCH v12 2/4] powerpc: Move arch independent ima kexec functions to drivers/of/kexec.c

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: <CAL_JsqJoyhOrWQiKq_q+7dpCj3MEsRtbGZ=SDxmeVswjQr-9sw@mail.gmail.com>
Date:   Fri, 18 Dec 2020 08:03:24 -0600
From:   Rob Herring <robh@...nel.org>
To:     Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>
Cc:     Mimi Zohar <zohar@...ux.ibm.com>,
        Thiago Jung Bauermann <bauerman@...ux.ibm.com>,
        "AKASHI, Takahiro" <takahiro.akashi@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Will Deacon <will@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Michael Ellerman <mpe@...erman.id.au>,
        James Morse <james.morse@....com>,
        Sasha Levin <sashal@...nel.org>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Frank Rowand <frowand.list@...il.com>,
        vincenzo.frascino@....com, Mark Rutland <mark.rutland@....com>,
        dmitry.kasatkin@...il.com, James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        Pavel Tatashin <pasha.tatashin@...een.com>,
        Allison Randal <allison@...utok.net>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Bhupesh Sharma <bhsharma@...hat.com>,
        Matthias Brugger <mbrugger@...e.com>,
        Hsin-Yi Wang <hsinyi@...omium.org>, tao.li@...o.com,
        Christophe Leroy <christophe.leroy@....fr>,
        Prakhar Srivastava <prsriva@...ux.microsoft.com>,
        balajib@...ux.microsoft.com, linux-integrity@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        devicetree@...r.kernel.org
Subject: Re: [PATCH v12 2/4] powerpc: Move arch independent ima kexec
 functions to drivers/of/kexec.c

On Fri, Dec 18, 2020 at 12:25 AM Lakshmi Ramasubramanian
<nramas@...ux.microsoft.com> wrote:
>
> On 12/17/20 2:01 PM, Rob Herring wrote:
>
> >
> > [...]
> >
> >>>> +#ifdef CONFIG_IMA_KEXEC
> >>>> +/**
> >>>> + * arch_ima_add_kexec_buffer - do arch-specific steps to add the IMA buffer
> >>>> + *
> >>>> + * @image: kimage struct to set IMA buffer data
> >>>> + * @load_addr: Starting address where IMA buffer is loaded at
> >>>> + * @size: Number of bytes in the IMA buffer
> >>>> + *
> >>>> + * Architectures should use this function to pass on the IMA buffer
> >>>> + * information to the next kernel.
> >>>> + *
> >>>> + * Return: 0 on success, negative errno on error.
> >>>> + */
> >>>> +int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
> >>>> +                          size_t size)
> >>>
> >>> This should be a static inline in asm/kexec.h.
> >>
> >> arch_ima_add_kexec_buffer() is identical for powerpc and arm64.
> >> Would it be better to "static inline" this function in "of.h" instead of
> >> duplicating it in "asm/kexec.h" for powerpc and arm64?
> >
> > No, think about what it is specific to and place it there. It has
> > nothing to do with DT really. All it is is a wrapper to access the
> > struct members in kimage_arch. So it belongs where they are declared.
> > Now perhaps ima_buffer_addr and ima_buffer_size shouldn't be arch
> > specific, but that's a separate issue.
> >
>
> Since "struct kimage" definition is not available in "asm/kexec.h",
> defining arch_ima_add_kexec_buffer() in this header file results in the
> following build error:
>
> ./arch/powerpc/include/asm/kexec.h: In function 'arch_ima_add_kexec_buffer':
> ./arch/powerpc/include/asm/kexec.h:139:7: error: 'struct kimage' has no
> member named 'arch'
>    139 |  image->arch.ima_buffer_addr = load_addr;
>
> I think it would be appropriate to make arch_ima_add_kexec_buffer() a
> static inline function in "security/integrity/ima/ima_kexec.c" - the
> only file where this function is used.

Even better. It doesn't need to be 'inline' then. The compiler will
inline it without.

Rob