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

[Xi Ruoyao <xry111@xry111.site>]

On Thu, 2022-09-15 at 09:47 +0800, liuzhensong wrote:
> > How about this?  We don't need to write into the GOT if
> > R_LARCH_RELATIVE
> > or R_LARCH_IRELATIVE will be used:
>  > "We don't need to write into the GOT if R_LARCH_RELATIVE or 
> R_LARCH_IRELATIVE will be used:"
> 
> Not only this, you can refer to the implementation of the function 
> _bfd_elf_allocate_ifunc_dyn_relocs for details.

I don't think _bfd_elf_allocate_ifunc_dyn_relocs (or
local_allocate_ifunc_dyn_relocs for us) and
loongarch_elf_relocate_section are strictly aligned.  We need to
allocate the space for GOT entry, but no need to fill in the content if
the entry will be resolved by R_LARCH_RELATIVE or R_LARCH_IRELATIVE
because they are RELA and the "origin" value of the entry is not used
during the resolution.

By the way, the code paths for REL can be also removed from
local_allocate_ifunc_dyn_relocs because LoongArch does not use REL at
all.

And the change passes ld testsuite, the resulted ld passes glibc
testsuite.

> > 
> > diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> > index a9bb66a1e04..1e8ecb2b8e2 100644
> > --- a/bfd/elfnn-loongarch.c
> > +++ b/bfd/elfnn-loongarch.c
> > @@ -3129,6 +3129,7 @@ loongarch_elf_relocate_section (bfd
> > *output_bfd, struct bfd_link_info *info,
> >               BFD_ASSERT (rel->r_addend == 0);
> >    
> >               bfd_vma got_off = 0;
> > +             bool fill_got_entry = true;
> >               if (h != NULL)
> >                 {
> >                   /* GOT ref or ifunc.  */
> > @@ -3141,6 +3142,10 @@ loongarch_elf_relocate_section (bfd
> > *output_bfd, struct bfd_link_info *info,
> >                   if (h->got.offset == MINUS_ONE && h->type ==
> > STT_GNU_IFUNC)
> >                     {
> >                       bfd_vma idx;
> > +
> > +                     /* An IFUNC is always resolved at runtime.  */
> > +                     fill_got_entry = false;
> > +
> >                       if (htab->elf.splt != NULL)
> >                         {
> >                           idx = (h->plt.offset - PLT_HEADER_SIZE)
> > @@ -3177,6 +3182,7 @@ loongarch_elf_relocate_section (bfd
> > *output_bfd, struct bfd_link_info *info,
> >                           rela.r_addend = relocation;
> >                           loongarch_elf_append_rela (output_bfd,
> >                                                      htab-
> > >elf.srelgot, &rela);
> > +                         fill_got_entry = false;
> >                         }
> >                       h->got.offset |= 1;
> >                     }
> > @@ -3197,12 +3203,14 @@ loongarch_elf_relocate_section (bfd
> > *output_bfd, struct bfd_link_info *info,
> >                           rela.r_addend = relocation;
> >                           loongarch_elf_append_rela (output_bfd,
> >                                                      htab-
> > >elf.srelgot, &rela);
> > +                         fill_got_entry = false;
> >                         }
> >                       local_got_offsets[r_symndx] |= 1;
> >                     }
> >                 }
> >    
> > -             bfd_put_NN (output_bfd, relocation, got->contents +
> > got_off);
> > +             if (fill_got_entry)
> > +               bfd_put_NN (output_bfd, relocation, got->contents +
> > got_off);
> >    
> >               relocation = got_off + sec_addr (got);
> >             }
> > 

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University