PR26569, R_RISCV_RVC_JUMP results in buffer overflow

Nelson Chu nelson.chu@sifive.com
Thu Sep 17 10:10:12 GMT 2020


Hi Alan,

LGTM, thanks for fixing them.  I have quickly tested my commonly used
toolchain (rv32i/rv64gc), and everything looks good for now.  But I'm
not a riscv FSF binutils maintainer, so I can not approve anything.
Just to mention that we may also encounter the similar size problems
in the future, and thank you for fixing it now.

Nelson

On Thu, Sep 17, 2020 at 4:20 PM Alan Modra via Binutils
<binutils@sourceware.org> 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?
>
> 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 */
>          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)
> +
>  /* 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
>
> --
> Alan Modra
> Australia Development Lab, IBM


More information about the Binutils mailing list