Re: [PATCH] LoongArch: fix .got.plt dislocation in static PDE

Hi Ruoyao,

Some comments below.


在 2026/8/2 12:07, Xi Ruoyao 写道:
> The code path handling R_LARCH_GOT{64*,{,_PC}_LO12} miscalculated the
> address of the .got.plt entry: it always counted the two .got.plt
> entries reserved for ld.so, but in static PDE those two entries do not
> exist (as in static PDE the PLT is solely for ifunc).  Thus all
> .got.plt entries dislocated for two slots.
>
> Those affected relocations are practically always used together with
> the R_LARCH_GOT_{PC_,}HI20 relocation.  In 2.46 and earlier releases,
> R_LARCH_GOT_{PC_,}HI20 set pointer_equality_needed, which caused the
> R_LARCH_GOT_* relocs resolved to a .got entry (targeting the PLT stub)
> instead of .got.plt in static PDE, thus the error was latent.  But in
> the 2.47 release R_LARCH_GOT_{PC_,}HI20 no longer sets
> pointer_equality_needed so the error became exploitable.
>
> Fix the issue by factoring out the correct .got.plt entry addressing
> logic handling R_LARCH_GOT_{PC_,}HI20 into a subroutine and replace the
> incorrect logic for R_LARCH_GOT{64*,{,_PC}_LO12} with a call to the
> subroutine.
>
> Signed-off-by: Xi Ruoyao <[email protected]>
> ---
>   bfd/elfnn-loongarch.c                         | 46 ++++++-------------
>   ld/testsuite/ld-loongarch-elf/ifunc.exp       |  8 ++++
>   ld/testsuite/ld-loongarch-elf/static-ifunc.c  | 27 +++++++++++
>   .../ld-loongarch-elf/static-ifunc.out         |  1 +
>   4 files changed, 51 insertions(+), 31 deletions(-)
>   create mode 100644 ld/testsuite/ld-loongarch-elf/static-ifunc.c
>   create mode 100644 ld/testsuite/ld-loongarch-elf/static-ifunc.out
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index 4bdc3f26285..658c5b93e08 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -3513,6 +3513,19 @@ loongarch_resolve_pcrel_lo_relocs (loongarch_pcrel_relocs *p)
>     return true;
>   }
>   
> +static bfd_vma
> +ifunc_got_off (struct elf_link_hash_table *htab,
> +	       struct elf_link_hash_entry *h)
> +{
> +  bfd_vma idx =
> +    (h->plt.offset - (htab->splt ? PLT_HEADER_SIZE : 0)) / PLT_ENTRY_SIZE;
> +
> +  return sec_addr (htab->sgotplt)


Would it make sense to use htab->igotplt when htab->splt is NULL?

Just like:

return sec_addr (htab->splt ? htab->sgotplt : htab->igotplt)


> +	 + (htab->splt ? GOTPLT_HEADER_SIZE : 0)
> +	 + (idx * GOT_ENTRY_SIZE)
> +	 - sec_addr (htab->sgot);
> +}
> +
>   static int
>   loongarch_elf_relocate_section (struct bfd_link_info *info,
>   				bfd *input_bfd, asection *input_section,
> @@ -4539,25 +4552,7 @@ loongarch_elf_relocate_section (struct bfd_link_info *info,
>   		  /* Hidden symbol not has got entry,
>   		   * only got.plt entry so it is (plt - got).  */
>   		  if (h->got.offset == MINUS_ONE && h->type == STT_GNU_IFUNC)
> -		    {
> -		      bfd_vma idx;
> -		      if (htab->elf.splt != NULL)
> -			{
> -			  idx = (h->plt.offset - PLT_HEADER_SIZE)
> -			    / PLT_ENTRY_SIZE;
> -			  got_off = sec_addr (htab->elf.sgotplt)
> -			    + GOTPLT_HEADER_SIZE
> -			    + (idx * GOT_ENTRY_SIZE)
> -			    - sec_addr (htab->elf.sgot);
> -			}
> -		      else
> -			{
> -			  idx = h->plt.offset / PLT_ENTRY_SIZE;
> -			  got_off = sec_addr (htab->elf.sgotplt)
> -			    + (idx * GOT_ENTRY_SIZE)
> -			    - sec_addr (htab->elf.sgot);
> -			}
> -		    }
> +		    got_off = ifunc_got_off (&htab->elf, h);
>   
>   		  if ((h->got.offset & 1) == 0)
>   		    {
> @@ -4638,18 +4633,7 @@ loongarch_elf_relocate_section (struct bfd_link_info *info,
>   		got_off = local_got_offsets[r_symndx] & (~(bfd_vma)1);
>   
>   	      if (h && h->got.offset == MINUS_ONE && h->type == STT_GNU_IFUNC)
> -		{
> -		  bfd_vma idx;
> -		  if (htab->elf.splt != NULL)
> -		    idx = (h->plt.offset - PLT_HEADER_SIZE) / PLT_ENTRY_SIZE;
> -		  else
> -		    idx = h->plt.offset / PLT_ENTRY_SIZE;
> -
> -		  got_off = sec_addr (htab->elf.sgotplt)
> -		    + GOTPLT_HEADER_SIZE
> -		    + (idx * GOT_ENTRY_SIZE)
> -		    - sec_addr (htab->elf.sgot);
> -		}
> +		got_off = ifunc_got_off (&htab->elf, h);
>   
>   	      relocation = got_off + sec_addr (got);
>   	    }
> diff --git a/ld/testsuite/ld-loongarch-elf/ifunc.exp b/ld/testsuite/ld-loongarch-elf/ifunc.exp
> index da33eeb8061..f6c97ae2019 100644
> --- a/ld/testsuite/ld-loongarch-elf/ifunc.exp
> +++ b/ld/testsuite/ld-loongarch-elf/ifunc.exp
> @@ -30,5 +30,13 @@ if [istarget loongarch*-*-*] {
>   	    "attr-ifunc-4" \
>   	    "attr-ifunc-4.out" \
>   	] \
> +	[list \
> +	    "Run static-ifunc" \
> +	    "-static" \
> +	    "" \
> +	    {static-ifunc.c} \
> +	    "static-ifunc" \
> +	    "static-ifunc.out" \
> +	] \
>       ]
>   }
> diff --git a/ld/testsuite/ld-loongarch-elf/static-ifunc.c b/ld/testsuite/ld-loongarch-elf/static-ifunc.c
> new file mode 100644
> index 00000000000..c0b1788df7c
> --- /dev/null
> +++ b/ld/testsuite/ld-loongarch-elf/static-ifunc.c
> @@ -0,0 +1,27 @@
> +int
> +f1 ()
> +{
> +  return 42;
> +}
> +int
> +f2 ()
> +{
> +  return 47;
> +}
> +
> +void *
> +fx ()
> +{
> +  return f1;
> +}
> +
> +[[gnu::ifunc ("fx")]] int f ();
> +
> +int
> +main ()
> +{
> +  int (*p) () = f;
> +  asm ("# prevent optimization" : "+r"(p));
> +  __builtin_printf ("%d\n", p ());
> +  return 0;
> +}
> diff --git a/ld/testsuite/ld-loongarch-elf/static-ifunc.out b/ld/testsuite/ld-loongarch-elf/static-ifunc.out
> new file mode 100644
> index 00000000000..d81cc0710eb
> --- /dev/null
> +++ b/ld/testsuite/ld-loongarch-elf/static-ifunc.out
> @@ -0,0 +1 @@
> +42