linux-kernel - Re: [PATCH v4 07/12] ppc64/kexec_file: add support to relocate purgatory

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:   Thu, 23 Jul 2020 16:47:31 +1000
From:   Michael Ellerman <mpe@...erman.id.au>
To:     Hari Bathini <hbathini@...ux.ibm.com>,
        Andrew Morton <akpm@...ux-foundation.org>
Cc:     kernel test robot <lkp@...el.com>, Pingfan Liu <piliu@...hat.com>,
        Kexec-ml <kexec@...ts.infradead.org>,
        Mimi Zohar <zohar@...ux.ibm.com>,
        Nayna Jain <nayna@...ux.ibm.com>,
        Petr Tesarik <ptesarik@...e.cz>,
        Mahesh J Salgaonkar <mahesh@...ux.ibm.com>,
        Sourabh Jain <sourabhjain@...ux.ibm.com>,
        lkml <linux-kernel@...r.kernel.org>,
        linuxppc-dev <linuxppc-dev@...abs.org>,
        Eric Biederman <ebiederm@...ssion.com>,
        Thiago Jung Bauermann <bauerman@...ux.ibm.com>,
        Dave Young <dyoung@...hat.com>, Vivek Goyal <vgoyal@...hat.com>
Subject: Re: [PATCH v4 07/12] ppc64/kexec_file: add support to relocate purgatory

Hari Bathini <hbathini@...ux.ibm.com> writes:
> On 22/07/20 9:55 am, Michael Ellerman wrote:
>> Hari Bathini <hbathini@...ux.ibm.com> writes:
>>> Right now purgatory implementation is only minimal. But if purgatory
>>> code is to be enhanced to copy memory to the backup region and verify
>>> sha256 digest, relocations may have to be applied to the purgatory.
>>> So, add support to relocate purgatory in kexec_file_load system call
>>> by setting up TOC pointer and applying RELA relocations as needed.
>>>
>>> Reported-by: kernel test robot <lkp@...el.com>
>>> [lkp: In v1, 'struct mem_sym' was declared in parameter list]
>>> Signed-off-by: Hari Bathini <hbathini@...ux.ibm.com>
>>> ---
>>>
>>> * Michael, can you share your opinion on the below:
>>>     - https://lore.kernel.org/patchwork/patch/1272027/
>>>     - My intention in cover note.
>> 
>> It seems like a lot of complexity for little benefit.
>> 
>> AFAICS your final purgatory_64.c is only 36 lines, and all it does is a
>> single (open coded) memcpy().
>> 
>> It seems like we could write that in not many more lines of assembler
>> and avoid all this code.
>
> Hi Michael,
>
> I am not sure if you would agree with me on this, but I am looking at the
> purgatory code as work in progress. As mentioned in the cover note, I intend
> to add log messaging, sha256 verification into purgatory. And also change it
> to position independent executable after moving common purgatory code (sha256
> verification) to arch-independent code.

I've never understood the desire to put more logic into purgatory. It's
the absolute worst place to detect that something's gone wrong, because
we have no facilities in there to do anything useful. We don't even know
what platform we're on.

> When I initially took this up, I wanted to add all the above changes too, but
> cut down on it, in the interest of time, first to get kdump (kexec -s -p)
> working in v5.9 merge window.
>
> But as the logic in patches 07/12 & 08/12 has been tested in kexec-tools code
> a lot of times and there are unlikely to be any changes to them except for
> __kexec_do_relocs() function (afaics), when -PIE would be used, I submitted them.
> With patch 09/12, I tried for a change that uses relocations while is minimal
> for now.
>
> Would you prefer it to be absolutely minimal by dropping patches 7 & 8 for
> now and writing the backup data copy code in assembler?

Yes please.

cheers