Re: [PATCH 1/2] LoongArch: Avoid heap-buffer-overflow in loongarch_elf_relocate_section

[liuzhensong <liuzhensong@loongson.cn>]

[-- Attachment #1: Type: text/plain, Size: 3868 bytes --]


在 2022/9/13 下午11:44, Xi Ruoyao 写道:
> If a and b are different sections, we cannot access something in b with
> "a->contents + (offset from a)" because "a->contents" and "b->contents"
> are heap buffers allocated separately, not slices of a large buffer.
>
> The issue was found during an attempt to add static-pie support to the
> toolchain with ASAN.

Can you provide compile parameters?

> ---
>   bfd/elfnn-loongarch.c | 18 ++++++++----------
>   1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index ed42b8b6770..4b408b1db72 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -3128,6 +3128,7 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
>   	      unresolved_reloc = false;
>   	      BFD_ASSERT (rel->r_addend == 0);
>   
> +	      asection *my_got = got;
>   	      bfd_vma got_off = 0;
>   	      if (h != NULL)
>   		{
> @@ -3145,17 +3146,14 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
>   			{
>   			  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);
> +			  my_got = htab->elf.sgotplt;
> +			  got_off = GOTPLT_HEADER_SIZE + idx * GOT_ENTRY_SIZE;
>   			}
>   		      else
>   			{
>   			  idx = h->plt.offset / PLT_ENTRY_SIZE;
> -			  got_off = sec_addr (htab->elf.sgotplt)
> -			    + (idx * GOT_ENTRY_SIZE)
> -			    - sec_addr (htab->elf.sgot);
> +			  my_got = htab->elf.sgotplt;
> +			  got_off = idx * GOT_ENTRY_SIZE;
>   			}
>   		    }
>   
> @@ -3172,7 +3170,7 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
>   			  && SYMBOL_REFERENCES_LOCAL (info, h))
>   			{
>   			  Elf_Internal_Rela rela;
> -			  rela.r_offset = sec_addr (got) + got_off;
> +			  rela.r_offset = sec_addr (my_got) + got_off;
>   			  rela.r_info = ELFNN_R_INFO (0, R_LARCH_RELATIVE);
>   			  rela.r_addend = relocation;
>   			  loongarch_elf_append_rela (output_bfd,
> @@ -3202,9 +3200,9 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
>   		    }
>   		}
>   
> -	      bfd_put_NN (output_bfd, relocation, got->contents + got_off);
> +	      bfd_put_NN (output_bfd, relocation, my_got->contents + got_off);
>   
> -	      relocation = got_off + sec_addr (got);
> +	      relocation = got_off + sec_addr (my_got);
>   	    }
>   
>   	  if (r_type == R_LARCH_GOT_PC_HI20)


This may be the reason of overflow.

Shouldn't write to got table when using hidden ifunc.


diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index ed42b8b6770..9278faa91aa 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -3179,6 +3179,7 @@ loongarch_elf_relocate_section (bfd *output_bfd, 
struct bfd_link_info *info,
htab->elf.srelgot, &rela);
                         }
                       h->got.offset |= 1;
+                     bfd_put_NN (output_bfd, relocation, got->contents 
+ got_off);
                     }
                 }
               else
@@ -3200,10 +3201,9 @@ loongarch_elf_relocate_section (bfd *output_bfd, 
struct bfd_link_info *info,
                         }
                       local_got_offsets[r_symndx] |= 1;
                     }
+                 bfd_put_NN (output_bfd, relocation, got->contents + 
got_off);
                 }

-             bfd_put_NN (output_bfd, relocation, got->contents + got_off);
-
               relocation = got_off + sec_addr (got);
             }