From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from loongson.cn (mail.loongson.cn [114.242.206.163]) by sourceware.org (Postfix) with ESMTP id 7F143385802A for ; Thu, 15 Sep 2022 03:54:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7F143385802A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=loongson.cn Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=loongson.cn Received: from [10.20.4.152] (unknown [10.20.4.152]) by localhost.localdomain (Coremail) with SMTP id AQAAf8Bx32vjoSJjc7oZAA--.27985S3; Thu, 15 Sep 2022 11:54:12 +0800 (CST) Subject: Re: [PATCH 1/2] LoongArch: Avoid heap-buffer-overflow in loongarch_elf_relocate_section To: Xi Ruoyao , binutils@sourceware.org Cc: Chenghua Xu , Lulu Cheng , Wang Xuerui References: <20220913154414.554861-1-xry111@xry111.site> <20220913154414.554861-2-xry111@xry111.site> <7f2c274b80fa79296005edaf52036745510a83fd.camel@xry111.site> <6f3d0d05-c7a2-b4cf-2a3c-abb1f343ff86@loongson.cn> <527fb03481edd9f58a938ecaf1583051cce06ac0.camel@xry111.site> From: liuzhensong Message-ID: <6fc2821d-5db6-6645-1d5f-8b89eb071fde@loongson.cn> Date: Thu, 15 Sep 2022 11:54:11 +0800 User-Agent: Mozilla/5.0 (X11; Linux mips64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <527fb03481edd9f58a938ecaf1583051cce06ac0.camel@xry111.site> Content-Type: multipart/alternative; boundary="------------CAE2C741CBD352A9FE48F11F" Content-Language: en-US X-CM-TRANSID:AQAAf8Bx32vjoSJjc7oZAA--.27985S3 X-Coremail-Antispam: 1UD129KBjvJXoWxZFW7Jw1fGrykJF4DKrWDtwb_yoWrZF15pr 18Jr1UJryUJr18Jr1UJr1UJryUJr1UJw1UJr1UJF1UJr1UJr1jqr1UXr1jgr1UJr48Jr1U Jr1UJr1UZr1UArUanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDU0xBIdaVrnRJUUUv214x267AKxVWUJVW8JwAFc2x0x2IEx4CE42xK8VAvwI8IcIk0 rVWrJVCq3wAFIxvE14AKwVWUJVWUGwA2ocxC64kIII0Yj41l84x0c7CEw4AK67xGY2AK02 1l84ACjcxK6xIIjxv20xvE14v26ryj6F1UM28EF7xvwVC0I7IYx2IY6xkF7I0E14v26F4j 6r4UJwA2z4x0Y4vEx4A2jsIE14v26r4UJVWxJr1l84ACjcxK6I8E87Iv6xkF7I0E14v26r xl6s0DM2AIxVAIcxkEcVAq07x20xvEncxIr21lYx0E2Ix0cI8IcVAFwI0_Jr0_Jr4lYx0E x4A2jsIE14v26r1j6r4UMcvjeVCFs4IE7xkEbVWUJVW8JwACjcxG0xvEwIxGrwACjI8F5V A0II8E6IAqYI8I648v4I1l7480Y4vEI4kI2Ix0rVAqx4xJMxk0xIA0c2IEe2xFo4CEbIxv r21lc2xSY4AK6svPMxAIw28IcxkI7VAKI48JMxC20s026xCaFVCjc4AY6r1j6r4UMI8I3I 0E5I8CrVAFwI0_JrI_JrWlx2IqxVCjr7xvwVAFwI0_JrI_JrWlx4CE17CEb7AF67AKxVWU AVWUtwCIc40Y0x0EwIxGrwCI42IY6xIIjxv20xvE14v26r1j6r1xMIIF0xvE2Ix0cI8IcV CY1x0267AKxVWUJVW8JwCI42IY6xAIw20EY4v20xvaj40_Jr0_JF4lIxAIcVC2z280aVAF wI0_Jr0_Gr1lIxAIcVC2z280aVCY1x0267AKxVW8JVW8JrUvcSsGvfC2KfnxnUUI43ZEXa 7VUb_gA7UUUUU== X-CM-SenderInfo: holx6xphqv003j6o00pqjv00gofq/ X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00,BODY_8BITS,GIT_PATCH_0,HTML_MESSAGE,KAM_DMARC_STATUS,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: This is a multi-part message in MIME format. --------------CAE2C741CBD352A9FE48F11F Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit 在 2022/9/15 上午10:56, Xi Ruoyao 写道: > 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. Some reasons of overflow. > > 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.  */ Not correct, anything can be filled if h->got.offset != MINUS_ONE. There is no space for ifunc to fill got. >>> +                     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); >>>             } >>> --------------CAE2C741CBD352A9FE48F11F--