PR26569, R_RISCV_RVC_JUMP results in buffer overflow
Palmer Dabbelt
palmer@dabbelt.com
Sat Sep 26 16:30:40 GMT 2020
On Sun, 20 Sep 2020 17:15:26 PDT (-0700), amodra@gmail.com wrote:
> On Sat, Sep 19, 2020 at 01:53:39PM -0700, Palmer Dabbelt wrote:
>> On Thu, 17 Sep 2020 01:20:18 PDT (-0700), amodra@gmail.com wrote:
>> > HOWTO (R_RISCV_ALIGN, /* type */
>> > 0, /* rightshift */
>> > - 2, /* size */
>> > + 3, /* size */
>>
>> Can't these be up to a page in size?
>
> These relocs are special, and can't be handled by the howto struct and
> generic code without implementing a special_function. So what is in
> the howto doesn't matter very much but this "size" agrees with
> "bitsize", "3" being the value to say the number of bytes is zero.
>
>> > 0, /* bitsize */
>> > FALSE, /* pc_relative */
>> > 0, /* bitpos */
>> > --- 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?
>
> Almost. There needs to be a frag_grow(8) before auipc is output.
> Otherwise if you're unlucky the frag runs out of room after the auipc
> and you hit the same "fixup not contained within frag".
>
>> 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. */
>
> I've committed the following.
Thanks!
> ----
>
> 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. Fixed by
> changing the frag handling a little.
>
> 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.
>
> 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 (append_insn): Don't tie off frags at CALL
> relocs.
> (riscv_call): Tie them off after the jalr.
> (md_apply_fix): Zero fx_size of RELAX fixup.
>
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index cfdd867e0d..e5adea57b1 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 */
> 0, /* bitsize */
> FALSE, /* pc_relative */
> 0, /* bitpos */
> @@ -643,13 +643,13 @@ static reloc_howto_type howto_table[] =
> FALSE, /* partial_inplace */
> 0, /* src_mask */
> 0, /* dst_mask */
> - TRUE), /* pcrel_offset */
> + FALSE), /* pcrel_offset */
>
> /* 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..eb31e42a2e 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)
> @@ -1311,8 +1309,13 @@ static void
> riscv_call (int destreg, int tempreg, expressionS *ep,
> bfd_reloc_code_real_type reloc)
> {
> + /* Ensure the jalr is emitted to the same frag as the auipc. */
> + frag_grow (8);
> macro_build (ep, "auipc", "d,u", tempreg, reloc);
> macro_build (NULL, "jalr", "d,s", destreg, tempreg);
> + /* See comment at end of append_insn. */
> + frag_wane (frag_now);
> + frag_new (0);
> }
>
> /* Load an integer constant into a register. */
> @@ -3031,6 +3034,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;
> }
> }
More information about the Binutils
mailing list