[PATCH 1/1] s390/kexec: handle R_390_PLT32DBL reloc entries in arch_kexec_do_relocs()

[PATCH 1/1] s390/kexec: handle R_390_PLT32DBL reloc entries in arch_kexec_do_relocs()

[Alexander Egorenkov]

Starting with gcc 11.3, the C compiler will generate PLT-relative function
calls even if they are local and do not require it. Later on during linking,
the linker will replace all PLT-relative calls to local functions with
PC-relative ones. Unfortunately, the purgatory code of kexec/kdump is
not being linked as a regular executable or shared library would have been,
and therefore, all PLT-relative addresses remain in the generated purgatory
object code unresolved. This leads to the situation where the purgatory
code is being executed during kdump with all PLT-relative addresses
unresolved. And this results in endless loops within the purgatory code.

Furthermore, the clang C compiler has always behaved like described above
and this commit should fix kdump for kernels built with the latter.

Because the purgatory code is no regular executable or shared library,
contains only calls to local functions and has no PLT, all R_390_PLT32DBL
relocation entries can be resolved just like a R_390_PC32DBL one.

* https://refspecs.linuxfoundation.org/ELF/zSeries/lzsabi0_zSeries/x1633.html#AEN1699

Relocation entries of purgatory code generated with gcc 11.3
------------------------------------------------------------

$ readelf -r linux/arch/s390/purgatory/purgatory.o

Relocation section '.rela.text' at offset 0x370 contains 5 entries:
  Offset          Info           Type           Sym. Value    Sym. Name + Addend
00000000005c  000c00000013 R_390_PC32DBL     0000000000000000 purgatory_sha_regions + 2
00000000007a  000d00000014 R_390_PLT32DBL    0000000000000000 sha256_update + 2
00000000008c  000e00000014 R_390_PLT32DBL    0000000000000000 sha256_final + 2
000000000092  000800000013 R_390_PC32DBL     0000000000000000 .LC0 + 2
0000000000a0  000f00000014 R_390_PLT32DBL    0000000000000000 memcmp + 2

Relocation entries of purgatory code generated with gcc 11.2
------------------------------------------------------------

$ readelf -r linux/arch/s390/purgatory/purgatory.o

Relocation section '.rela.text' at offset 0x368 contains 5 entries:
  Offset          Info           Type           Sym. Value    Sym. Name + Addend
00000000005c  000c00000013 R_390_PC32DBL     0000000000000000 purgatory_sha_regions + 2
00000000007a  000d00000013 R_390_PC32DBL     0000000000000000 sha256_update + 2
00000000008c  000e00000013 R_390_PC32DBL     0000000000000000 sha256_final + 2
000000000092  000800000013 R_390_PC32DBL     0000000000000000 .LC0 + 2
0000000000a0  000f00000013 R_390_PC32DBL     0000000000000000 memcmp + 2

Signed-off-by: Alexander Egorenkov <egorenar@linux.ibm.com>
Reported-by: Tao Liu <ltao@redhat.com>
Suggested-by: Philipp Rudo <prudo@redhat.com>
---
 arch/s390/kernel/machine_kexec_reloc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/s390/kernel/machine_kexec_reloc.c b/arch/s390/kernel/machine_kexec_reloc.c
index b7182cec48dc..fea6fcae2270 100644
--- a/arch/s390/kernel/machine_kexec_reloc.c
+++ b/arch/s390/kernel/machine_kexec_reloc.c
@@ -38,6 +38,7 @@ int arch_kexec_do_relocs(int r_type, void *loc, unsigned long val,
 		*(u16 *)loc = (val - addr) >> 1;
 		break;
 	case R_390_PC32DBL:	/* PC relative 32 bit shifted by 1.  */
+	case R_390_PLT32DBL:	/* 32 bit PC rel. PLT shifted by 1.  */
 		*(u32 *)loc = (val - addr) >> 1;
 		break;
 	case R_390_PC32:	/* PC relative 32 bit.	*/
-- 
2.31.1

Re: [PATCH 1/1] s390/kexec: handle R_390_PLT32DBL reloc entries in arch_kexec_do_relocs()

[Philipp Rudo]

Hi Alexander,

thanks for taking care of this so fast!

My personal approach was slightly different (see below). The idea
behind this was that arch_kexec_do_relocs is also used by others which
might have a PLT. For them your approach would mean an ABI breakage. On
the other hand in case the other users have the same problem they would
need to handle it for themselves. Not sure what's the better approach.

Thanks
Philipp

--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -326,6 +326,10 @@ int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
                addr = section->sh_addr + relas[i].r_offset;
 
                r_type = ELF64_R_TYPE(relas[i].r_info);
+
+               if (r_type == R_390_PLT32DBL)
+                   r_type = R_390_PC32DBL;
+
                arch_kexec_do_relocs(r_type, loc, val, addr);
        }
        return 0;

On Wed,  8 Dec 2021 11:58:01 +0100
Alexander Egorenkov <egorenar@linux.ibm.com> wrote:

> Starting with gcc 11.3, the C compiler will generate PLT-relative function
> calls even if they are local and do not require it. Later on during linking,
> the linker will replace all PLT-relative calls to local functions with
> PC-relative ones. Unfortunately, the purgatory code of kexec/kdump is
> not being linked as a regular executable or shared library would have been,
> and therefore, all PLT-relative addresses remain in the generated purgatory
> object code unresolved. This leads to the situation where the purgatory
> code is being executed during kdump with all PLT-relative addresses
> unresolved. And this results in endless loops within the purgatory code.
> 
> Furthermore, the clang C compiler has always behaved like described above
> and this commit should fix kdump for kernels built with the latter.
> 
> Because the purgatory code is no regular executable or shared library,
> contains only calls to local functions and has no PLT, all R_390_PLT32DBL
> relocation entries can be resolved just like a R_390_PC32DBL one.
> 
> * https://refspecs.linuxfoundation.org/ELF/zSeries/lzsabi0_zSeries/x1633.html#AEN1699
> 
> Relocation entries of purgatory code generated with gcc 11.3
> ------------------------------------------------------------
> 
> $ readelf -r linux/arch/s390/purgatory/purgatory.o
> 
> Relocation section '.rela.text' at offset 0x370 contains 5 entries:
>   Offset          Info           Type           Sym. Value    Sym. Name + Addend
> 00000000005c  000c00000013 R_390_PC32DBL     0000000000000000 purgatory_sha_regions + 2
> 00000000007a  000d00000014 R_390_PLT32DBL    0000000000000000 sha256_update + 2
> 00000000008c  000e00000014 R_390_PLT32DBL    0000000000000000 sha256_final + 2
> 000000000092  000800000013 R_390_PC32DBL     0000000000000000 .LC0 + 2
> 0000000000a0  000f00000014 R_390_PLT32DBL    0000000000000000 memcmp + 2
> 
> Relocation entries of purgatory code generated with gcc 11.2
> ------------------------------------------------------------
> 
> $ readelf -r linux/arch/s390/purgatory/purgatory.o
> 
> Relocation section '.rela.text' at offset 0x368 contains 5 entries:
>   Offset          Info           Type           Sym. Value    Sym. Name + Addend
> 00000000005c  000c00000013 R_390_PC32DBL     0000000000000000 purgatory_sha_regions + 2
> 00000000007a  000d00000013 R_390_PC32DBL     0000000000000000 sha256_update + 2
> 00000000008c  000e00000013 R_390_PC32DBL     0000000000000000 sha256_final + 2
> 000000000092  000800000013 R_390_PC32DBL     0000000000000000 .LC0 + 2
> 0000000000a0  000f00000013 R_390_PC32DBL     0000000000000000 memcmp + 2
> 
> Signed-off-by: Alexander Egorenkov <egorenar@linux.ibm.com>
> Reported-by: Tao Liu <ltao@redhat.com>
> Suggested-by: Philipp Rudo <prudo@redhat.com>
> ---
>  arch/s390/kernel/machine_kexec_reloc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/s390/kernel/machine_kexec_reloc.c b/arch/s390/kernel/machine_kexec_reloc.c
> index b7182cec48dc..fea6fcae2270 100644
> --- a/arch/s390/kernel/machine_kexec_reloc.c
> +++ b/arch/s390/kernel/machine_kexec_reloc.c
> @@ -38,6 +38,7 @@ int arch_kexec_do_relocs(int r_type, void *loc, unsigned long val,
>  		*(u16 *)loc = (val - addr) >> 1;
>  		break;
>  	case R_390_PC32DBL:	/* PC relative 32 bit shifted by 1.  */
> +	case R_390_PLT32DBL:	/* 32 bit PC rel. PLT shifted by 1.  */
>  		*(u32 *)loc = (val - addr) >> 1;
>  		break;
>  	case R_390_PC32:	/* PC relative 32 bit.	*/

Re: [PATCH 1/1] s390/kexec: handle R_390_PLT32DBL reloc entries in arch_kexec_do_relocs()

[Alexander Egorenkov]

Hi Philipp,

Philipp Rudo <prudo@redhat.com> writes:

> Hi Alexander,
>
> thanks for taking care of this so fast!
>
> My personal approach was slightly different (see below). The idea
> behind this was that arch_kexec_do_relocs is also used by others which
> might have a PLT. For them your approach would mean an ABI breakage. On
> the other hand in case the other users have the same problem they would
> need to handle it for themselves. Not sure what's the better approach.
>

I'm also fine with your proposal for the fix.

If i'm not mistaken, arch_kexec_do_relocs() is used in decompressor and
for purgatory at the moment, and both should have all relocs resolved.
Do we have more users of arch_kexec_do_relocs() ? That was the reason i
added the fix to arch_kexec_do_relocs().

Now i'm actually wondering why we don't have any issues with
decompressor, probably because we do the final link where all
PLT-relative addresses are fixed. Because i see plenty of R_390_PLT32DBL
in startup.o e.g.

Thanks for feedback
Regards
Alex 

Re: [PATCH 1/1] s390/kexec: handle R_390_PLT32DBL reloc entries in arch_kexec_do_relocs()

[Philipp Rudo]

Hi Alexander,

On Wed, 08 Dec 2021 18:42:05 +0100
Alexander Egorenkov <egorenar@linux.ibm.com> wrote:

> Hi Philipp,
> 
> Philipp Rudo <prudo@redhat.com> writes:
> 
> > Hi Alexander,
> >
> > thanks for taking care of this so fast!
> >
> > My personal approach was slightly different (see below). The idea
> > behind this was that arch_kexec_do_relocs is also used by others which
> > might have a PLT. For them your approach would mean an ABI breakage. On
> > the other hand in case the other users have the same problem they would
> > need to handle it for themselves. Not sure what's the better approach.
> >  
> 
> I'm also fine with your proposal for the fix.
> 
> If i'm not mistaken, arch_kexec_do_relocs() is used in decompressor and
> for purgatory at the moment, and both should have all relocs resolved.
> Do we have more users of arch_kexec_do_relocs() ? That was the reason i
> added the fix to arch_kexec_do_relocs().

no, there are no other users of arch_kexec_do_relocs other than
kexec_file and kaslr at the moment.

From a technical perspective your patch was totally fine for todays
use cases. My concern was more for a potential future user. But maybe I
was a little bit overcautious... 

> Now i'm actually wondering why we don't have any issues with
> decompressor, probably because we do the final link where all
> PLT-relative addresses are fixed. Because i see plenty of R_390_PLT32DBL
> in startup.o e.g.

IIUC, the culprit is the '-r' option in the LDFLAGS. With this option
the output can be used again as input _for ld_. The way I interpret
this is that ld can (and apparently does) use any internal convention
without complying with the ABI when this option is given.

kaslr doesn't use '-r' but a combination of -fPIE (CFLAGS) and -pie
(LDFLAGS) so they are not affected by ld's internal behavior. Maybe
it's worth investigating in the long run if the purgatory could be build
with -fPIE/-pie as well.

Thanks
Philipp