From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x429.google.com (mail-pf1-x429.google.com [IPv6:2607:f8b0:4864:20::429]) by sourceware.org (Postfix) with ESMTPS id 133F83851C39 for ; Sat, 19 Sep 2020 20:53:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 133F83851C39 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=dabbelt.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=palmer@dabbelt.com Received: by mail-pf1-x429.google.com with SMTP id k8so5767500pfk.2 for ; Sat, 19 Sep 2020 13:53:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dabbelt-com.20150623.gappssmtp.com; s=20150623; h=date:subject:in-reply-to:cc:from:to:message-id:mime-version :content-transfer-encoding; bh=jBUkMcnilI0ZRrmucOaL5Zg5XsOM0gWqa2og2O9T3Nc=; b=UiBQ/YpMJIDx05WihPSlQ2r2YupoohSN2AsH0zS6e1BnXM41P5DN58EMTtyySB+mMg F4/GdeBETjPutOdJWx0RgII77HTKgJtWiMO8jV8kftW3qmdiblwE4A+tJEDe9H2GCjsy U8bTV6eEi5IYpgbRNd+66KpFx4wemNU688HHZ26Tte+L17MMoravJEP7ir2i1gRUySqE 96GF8gVcLZ5lacOZAnaTXpEKFhA4LXpnPBjpXQr2BVQ6kxCmli/NPpwHFB2xle6VCRC1 WKEHAH5b0zWUy4b/oVmjQ4bJMHk85WlvTiDqcVJTF69rPHYfPGGGj2hDR2pvgDuC/rXO fWsA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:subject:in-reply-to:cc:from:to:message-id :mime-version:content-transfer-encoding; bh=jBUkMcnilI0ZRrmucOaL5Zg5XsOM0gWqa2og2O9T3Nc=; b=NZQjqGRpPcrjB5AZUtu6g7PkaeLoc86l74/+azvr7tQ2uNwTTMbawizcJsP7YH7eSD 2IGEOf6Ns6ZBrFKwUquYWa/+cD1dVhtQy2CJK2si7cPcyeGcBJCFp8H+bpWTIiJbK47e vOmacGUSnuoOV0s+0YJKmQ04Q9lurhErFLtDPWbZJ4PuNofabIqlelMb263rxlTkOMem 3QikWTPBBZT1vHoiL+h27fMToIkELCmCKhErYikvqxzai3WOiku5w4t1jbrht/ouci25 B/9xg9jgWUXNW/cF2QOD0abNH7U1w/sGVVr0cZilbSVcoWQmOvpNCgePTguHw5WdX6cM WHpg== X-Gm-Message-State: AOAM5307hNiMH5F+piWMAekNQKBrYedytQMe7okVUKkiYNasCGUrxIZu MawjVZ1k1RYRv1gP4S0uNqZGnp3sxS0ZBAyr X-Google-Smtp-Source: ABdhPJx1jNKo1PE86SxrTT88zygyxclZDAywqvmg/s76B9gSoLKD/0ZePB7PHHRm884hRtHZNY+S/A== X-Received: by 2002:a63:2f02:: with SMTP id v2mr31399576pgv.369.1600548819583; Sat, 19 Sep 2020 13:53:39 -0700 (PDT) Received: from localhost (76-210-143-223.lightspeed.sntcca.sbcglobal.net. [76.210.143.223]) by smtp.gmail.com with ESMTPSA id t24sm7153329pgo.51.2020.09.19.13.53.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 19 Sep 2020 13:53:39 -0700 (PDT) Date: Sat, 19 Sep 2020 13:53:39 -0700 (PDT) X-Google-Original-Date: Sat, 19 Sep 2020 13:53:36 PDT (-0700) Subject: Re: PR26569, R_RISCV_RVC_JUMP results in buffer overflow In-Reply-To: <20200917082018.GR5452@bubble.grove.modra.org> CC: binutils@sourceware.org, Andrew Waterman , Jim Wilson From: Palmer Dabbelt To: amodra@gmail.com Message-ID: Mime-Version: 1.0 (MHng) Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 19 Sep 2020 20:53:43 -0000 On Thu, 17 Sep 2020 01:20:18 PDT (-0700), amodra@gmail.com wrote: > This patch corrects "size" and "bitsize" in R_RISCV_RVC_* reloc howtos > so that elfnn-riscv.c:perform_relocation doesn't access past the end > of a section. I've also corrected "size" in the R_RISCV_CALL* reloc > howtos since these relocs apply to two consecutive instructions. That > caused fallout in the assembler with complaints about "fixup not > contained within frag" due to tc-riscv.c:append_insn finishing off a > frag after the auipc insn making up a "call" macro. Which is a little > rude since the CALL reloc also relocates the following jalr. I wasn't > game to change the frag handling, so worked around the error using > TC_FIX_SIZE_SLACK, and adjusted fx_size for the RELAX reloc following > a CALL. > > I've also changed R_RISCV_ALIGN and R_RISCV_TPREL_ADD marker reloc > howtos to look like R_RISCV_NONE, and corrected dst_mask for numerous > relocs, not that it matters very much. > > OK to apply? LGTM. There's some comments, but they seem amenable to a follow-on. Thanks! > > bfd/ > PR 26569 > * elfxx-riscv.c (howto_table): Correct size and bitsize of > R_RISCV_RVC_BRANCH, R_RISCV_RVC_JUMP, and R_RISCV_RVC_LUI. > Correct size for R_RISCV_TLS_DTPMOD32, R_RISCV_TLS_DTPREL32, > R_RISCV_CALL, and R_RISCV_CALL_PLT. Make R_RISCV_TPREL_ADD and > R_RISCV_ALIGN like R_RISCV_NONE. Correct dst_mask many relocs. > gas/ > * config/tc-riscv.c (md_apply_fix): Zero fx_size of RELAX fixup. > * config/tc-riscv.g (TC_FX_SIZE_SLACK): Define. > > diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c > index cfdd867e0d..181969521a 100644 > --- a/bfd/elfxx-riscv.c > +++ b/bfd/elfxx-riscv.c > @@ -69,7 +69,7 @@ static reloc_howto_type howto_table[] = > "R_RISCV_32", /* name */ > FALSE, /* partial_inplace */ > 0, /* src_mask */ > - MINUS_ONE, /* dst_mask */ > + 0xffffffff, /* dst_mask */ > FALSE), /* pcrel_offset */ > > /* 64 bit relocation. */ > @@ -99,7 +99,7 @@ static reloc_howto_type howto_table[] = > "R_RISCV_RELATIVE", /* name */ > FALSE, /* partial_inplace */ > 0, /* src_mask */ > - MINUS_ONE, /* dst_mask */ > + 0xffffffff, /* dst_mask */ > FALSE), /* pcrel_offset */ > > HOWTO (R_RISCV_COPY, /* type */ > @@ -133,7 +133,7 @@ static reloc_howto_type howto_table[] = > /* Dynamic TLS relocations. */ > HOWTO (R_RISCV_TLS_DTPMOD32, /* type */ > 0, /* rightshift */ > - 4, /* size */ > + 2, /* size */ > 32, /* bitsize */ > FALSE, /* pc_relative */ > 0, /* bitpos */ > @@ -142,7 +142,7 @@ static reloc_howto_type howto_table[] = > "R_RISCV_TLS_DTPMOD32", /* name */ > FALSE, /* partial_inplace */ > 0, /* src_mask */ > - MINUS_ONE, /* dst_mask */ > + 0xffffffff, /* dst_mask */ > FALSE), /* pcrel_offset */ > > HOWTO (R_RISCV_TLS_DTPMOD64, /* type */ > @@ -161,7 +161,7 @@ static reloc_howto_type howto_table[] = > > HOWTO (R_RISCV_TLS_DTPREL32, /* type */ > 0, /* rightshift */ > - 4, /* size */ > + 2, /* size */ > 32, /* bitsize */ > FALSE, /* pc_relative */ > 0, /* bitpos */ > @@ -170,7 +170,7 @@ static reloc_howto_type howto_table[] = > "R_RISCV_TLS_DTPREL32", /* name */ > TRUE, /* partial_inplace */ > 0, /* src_mask */ > - MINUS_ONE, /* dst_mask */ > + 0xffffffff, /* dst_mask */ > FALSE), /* pcrel_offset */ > > HOWTO (R_RISCV_TLS_DTPREL64, /* type */ > @@ -198,7 +198,7 @@ static reloc_howto_type howto_table[] = > "R_RISCV_TLS_TPREL32", /* name */ > FALSE, /* partial_inplace */ > 0, /* src_mask */ > - MINUS_ONE, /* dst_mask */ > + 0xffffffff, /* dst_mask */ > FALSE), /* pcrel_offset */ > > HOWTO (R_RISCV_TLS_TPREL64, /* type */ > @@ -254,7 +254,7 @@ static reloc_howto_type howto_table[] = > /* 32-bit PC-relative function call (AUIPC/JALR). */ > HOWTO (R_RISCV_CALL, /* type */ > 0, /* rightshift */ > - 2, /* size */ > + 4, /* size */ > 64, /* bitsize */ > TRUE, /* pc_relative */ > 0, /* bitpos */ > @@ -270,7 +270,7 @@ static reloc_howto_type howto_table[] = > /* Like R_RISCV_CALL, but not locally binding. */ > HOWTO (R_RISCV_CALL_PLT, /* type */ > 0, /* rightshift */ > - 2, /* size */ > + 4, /* size */ > 64, /* bitsize */ > TRUE, /* pc_relative */ > 0, /* bitpos */ > @@ -466,14 +466,14 @@ static reloc_howto_type howto_table[] = > /* TLS LE thread pointer usage. May be relaxed. */ > HOWTO (R_RISCV_TPREL_ADD, /* type */ > 0, /* rightshift */ > - 2, /* size */ > - 32, /* bitsize */ > + 3, /* size */ > + 0, /* bitsize */ > FALSE, /* pc_relative */ > 0, /* bitpos */ > complain_overflow_dont, /* complain_on_overflow */ > bfd_elf_generic_reloc, /* special_function */ > "R_RISCV_TPREL_ADD", /* name */ > - TRUE, /* partial_inplace */ > + FALSE, /* partial_inplace */ > 0, /* src_mask */ > 0, /* dst_mask */ > FALSE), /* pcrel_offset */ > @@ -490,7 +490,7 @@ static reloc_howto_type howto_table[] = > "R_RISCV_ADD8", /* name */ > FALSE, /* partial_inplace */ > 0, /* src_mask */ > - MINUS_ONE, /* dst_mask */ > + 0xff, /* dst_mask */ > FALSE), /* pcrel_offset */ > > /* 16-bit in-place addition, for local label subtraction. */ > @@ -505,7 +505,7 @@ static reloc_howto_type howto_table[] = > "R_RISCV_ADD16", /* name */ > FALSE, /* partial_inplace */ > 0, /* src_mask */ > - MINUS_ONE, /* dst_mask */ > + 0xffff, /* dst_mask */ > FALSE), /* pcrel_offset */ > > /* 32-bit in-place addition, for local label subtraction. */ > @@ -520,7 +520,7 @@ static reloc_howto_type howto_table[] = > "R_RISCV_ADD32", /* name */ > FALSE, /* partial_inplace */ > 0, /* src_mask */ > - MINUS_ONE, /* dst_mask */ > + 0xffffffff, /* dst_mask */ > FALSE), /* pcrel_offset */ > > /* 64-bit in-place addition, for local label subtraction. */ > @@ -550,7 +550,7 @@ static reloc_howto_type howto_table[] = > "R_RISCV_SUB8", /* name */ > FALSE, /* partial_inplace */ > 0, /* src_mask */ > - MINUS_ONE, /* dst_mask */ > + 0xff, /* dst_mask */ > FALSE), /* pcrel_offset */ > > /* 16-bit in-place addition, for local label subtraction. */ > @@ -565,7 +565,7 @@ static reloc_howto_type howto_table[] = > "R_RISCV_SUB16", /* name */ > FALSE, /* partial_inplace */ > 0, /* src_mask */ > - MINUS_ONE, /* dst_mask */ > + 0xffff, /* dst_mask */ > FALSE), /* pcrel_offset */ > > /* 32-bit in-place addition, for local label subtraction. */ > @@ -580,7 +580,7 @@ static reloc_howto_type howto_table[] = > "R_RISCV_SUB32", /* name */ > FALSE, /* partial_inplace */ > 0, /* src_mask */ > - MINUS_ONE, /* dst_mask */ > + 0xffffffff, /* dst_mask */ > FALSE), /* pcrel_offset */ > > /* 64-bit in-place addition, for local label subtraction. */ > @@ -633,7 +633,7 @@ static reloc_howto_type howto_table[] = > addend rounded up to the next power of two. */ > HOWTO (R_RISCV_ALIGN, /* type */ > 0, /* rightshift */ > - 2, /* size */ > + 3, /* size */ Can't these be up to a page in size? > 0, /* bitsize */ > FALSE, /* pc_relative */ > 0, /* bitpos */ > @@ -648,8 +648,8 @@ static reloc_howto_type howto_table[] = > /* 8-bit PC-relative branch offset. */ > HOWTO (R_RISCV_RVC_BRANCH, /* type */ > 0, /* rightshift */ > - 2, /* size */ > - 32, /* bitsize */ > + 1, /* size */ > + 16, /* bitsize */ > TRUE, /* pc_relative */ > 0, /* bitpos */ > complain_overflow_signed, /* complain_on_overflow */ > @@ -663,8 +663,8 @@ static reloc_howto_type howto_table[] = > /* 11-bit PC-relative jump offset. */ > HOWTO (R_RISCV_RVC_JUMP, /* type */ > 0, /* rightshift */ > - 2, /* size */ > - 32, /* bitsize */ > + 1, /* size */ > + 16, /* bitsize */ > TRUE, /* pc_relative */ > 0, /* bitpos */ > complain_overflow_dont, /* complain_on_overflow */ > @@ -678,8 +678,8 @@ static reloc_howto_type howto_table[] = > /* High 6 bits of 18-bit absolute address. */ > HOWTO (R_RISCV_RVC_LUI, /* type */ > 0, /* rightshift */ > - 2, /* size */ > - 32, /* bitsize */ > + 1, /* size */ > + 16, /* bitsize */ > FALSE, /* pc_relative */ > 0, /* bitpos */ > complain_overflow_dont, /* complain_on_overflow */ > @@ -807,7 +807,7 @@ static reloc_howto_type howto_table[] = > "R_RISCV_SET8", /* name */ > FALSE, /* partial_inplace */ > 0, /* src_mask */ > - MINUS_ONE, /* dst_mask */ > + 0xff, /* dst_mask */ > FALSE), /* pcrel_offset */ > > /* 16-bit in-place setting, for local label subtraction. */ > @@ -822,7 +822,7 @@ static reloc_howto_type howto_table[] = > "R_RISCV_SET16", /* name */ > FALSE, /* partial_inplace */ > 0, /* src_mask */ > - MINUS_ONE, /* dst_mask */ > + 0xffff, /* dst_mask */ > FALSE), /* pcrel_offset */ > > /* 32-bit in-place setting, for local label subtraction. */ > @@ -837,7 +837,7 @@ static reloc_howto_type howto_table[] = > "R_RISCV_SET32", /* name */ > FALSE, /* partial_inplace */ > 0, /* src_mask */ > - MINUS_ONE, /* dst_mask */ > + 0xffffffff, /* dst_mask */ > FALSE), /* pcrel_offset */ > > /* 32-bit PC relative. */ > @@ -852,7 +852,7 @@ static reloc_howto_type howto_table[] = > "R_RISCV_32_PCREL", /* name */ > FALSE, /* partial_inplace */ > 0, /* src_mask */ > - MINUS_ONE, /* dst_mask */ > + 0xffffffff, /* dst_mask */ > FALSE), /* pcrel_offset */ > }; > > diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c > index 9df6d3f415..e1f7673eb5 100644 > --- a/gas/config/tc-riscv.c > +++ b/gas/config/tc-riscv.c > @@ -3031,6 +3031,7 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg ATTRIBUTE_UNUSED) > fixP->fx_next = xmemdup (fixP, sizeof (*fixP), sizeof (*fixP)); > fixP->fx_next->fx_addsy = fixP->fx_next->fx_subsy = NULL; > fixP->fx_next->fx_r_type = BFD_RELOC_RISCV_RELAX; > + fixP->fx_next->fx_size = 0; > } > } > > diff --git a/gas/config/tc-riscv.h b/gas/config/tc-riscv.h > index 43d751ad0d..83cff8f2df 100644 > --- a/gas/config/tc-riscv.h > +++ b/gas/config/tc-riscv.h > @@ -77,6 +77,14 @@ extern int riscv_parse_long_option (const char *); > #define md_pre_output_hook riscv_pre_output_hook() > extern void riscv_pre_output_hook (void); > > +/* Call macro frags are tied off wrongly at the auipc which has an > + eight byte R_RISCV_CALL reloc covering both the auipc and > + immediately following jalr. For now, work around this by allowing > + CALL relocs to extend past the end of a frag. */ > +#define TC_FX_SIZE_SLACK(FIX) \ > + (((FIX)->fx_r_type == BFD_RELOC_RISCV_CALL \ > + || (FIX)->fx_r_type == BFD_RELOC_RISCV_CALL_PLT) ? 4 : 0) > + I haven't tried it yet, but I think something like this would eliminate the need for this slack? diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c index 9df6d3f415..cec235b986 100644 --- a/gas/config/tc-riscv.c +++ b/gas/config/tc-riscv.c @@ -1133,9 +1133,7 @@ append_insn (struct riscv_cl_insn *ip, expressionS *address_expr, optimized away or compressed by the linker during relaxation, to prevent the assembler from computing static offsets across such an instruction. This is necessary to get correct EH info. */ - if (reloc_type == BFD_RELOC_RISCV_CALL - || reloc_type == BFD_RELOC_RISCV_CALL_PLT - || reloc_type == BFD_RELOC_RISCV_HI20 + if (reloc_type == BFD_RELOC_RISCV_HI20 || reloc_type == BFD_RELOC_RISCV_PCREL_HI20 || reloc_type == BFD_RELOC_RISCV_TPREL_HI20 || reloc_type == BFD_RELOC_RISCV_TPREL_ADD) @@ -1313,6 +1311,8 @@ riscv_call (int destreg, int tempreg, expressionS *ep, { macro_build (ep, "auipc", "d,u", tempreg, reloc); macro_build (NULL, "jalr", "d,s", destreg, tempreg); + frag_wane(frag_now); + frag_new(0); } /* Load an integer constant into a register. */ > /* Let the linker resolve all the relocs due to relaxation. */ > #define tc_fix_adjustable(fixp) 0 > #define md_allow_local_subtract(l,r,s) 0