diff mbox series

[1/5] LoongArch: Reject R_LARCH_32 from becoming a runtime reloc in ELFCLASS64

Message ID 20240622100307.9498-2-xry111@xry111.site
State New
Headers
Series LoongArch: Add DT_RELR (packing relative relocs) support |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Xi Ruoyao June 22, 2024, 10:03 a.m. UTC 
  We were converting R_LARCH_32 to R_LARCH_RELATIVE for ELFCLASS64:

    $ cat t.s
    .data
    x:
        .4byte x
    $ as/as-new t.s -o t.o
    $ ld/ld-new -shared t.s
    $ objdump -R
    a.out:     file format elf64-loongarch

    DYNAMIC RELOCATION RECORDS
    OFFSET           TYPE              VALUE
    00000000000001a8 R_LARCH_RELATIVE  *ABS*+0x00000000000001a8

But this is just wrong: at runtime the dynamic linker will run
*(uintptr *)&x += load_address, causing an OOB write.

It does not make too much sense to support R_LARCH_32 in ELFCLASS64 or
R_LARCH_64 in ELFCLASS32 when creating a DLL or PIE.  And, if we keep it
as-is in the linked object, it'll be rejected by Glibc dynamic linker
anyway.  So we can just reject it like x86_64 and RISC-V.

Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---
 bfd/elfnn-loongarch.c                         | 23 ++++++++++++++++++-
 .../ld-loongarch-elf/ld-loongarch-elf.exp     |  1 +
 .../ld-loongarch-elf/r_larch_32_elf64.d       |  4 ++++
 .../ld-loongarch-elf/r_larch_32_elf64.s       |  3 +++
 4 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100644 ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d
 create mode 100644 ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s

Comments

Fangrui Song June 22, 2024, 5:31 p.m. UTC | #1 
On Sat, Jun 22, 2024 at 3:03 AM Xi Ruoyao <xry111@xry111.site> wrote:
>
> We were converting R_LARCH_32 to R_LARCH_RELATIVE for ELFCLASS64:
>
>     $ cat t.s
>     .data
>     x:
>         .4byte x
>     $ as/as-new t.s -o t.o
>     $ ld/ld-new -shared t.s
>     $ objdump -R
>     a.out:     file format elf64-loongarch
>
>     DYNAMIC RELOCATION RECORDS
>     OFFSET           TYPE              VALUE
>     00000000000001a8 R_LARCH_RELATIVE  *ABS*+0x00000000000001a8
>
> But this is just wrong: at runtime the dynamic linker will run
> *(uintptr *)&x += load_address, causing an OOB write.
>
> It does not make too much sense to support R_LARCH_32 in ELFCLASS64 or
> R_LARCH_64 in ELFCLASS32 when creating a DLL or PIE.  And, if we keep it
> as-is in the linked object, it'll be rejected by Glibc dynamic linker
> anyway.  So we can just reject it like x86_64 and RISC-V.
>
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>

LGTM.

DLL => DSO ?

> ---
>  bfd/elfnn-loongarch.c                         | 23 ++++++++++++++++++-
>  .../ld-loongarch-elf/ld-loongarch-elf.exp     |  1 +
>  .../ld-loongarch-elf/r_larch_32_elf64.d       |  4 ++++
>  .../ld-loongarch-elf/r_larch_32_elf64.s       |  3 +++
>  4 files changed, 30 insertions(+), 1 deletion(-)
>  create mode 100644 ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d
>  create mode 100644 ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index b1720760475..3a55ac93e20 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -2861,7 +2861,28 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
>               /* No alloc space of func allocate_dynrelocs.  */
>               if (unresolved_reloc
>                   && !(h && (h->is_weakalias || !h->dyn_relocs)))
> -               loongarch_elf_append_rela (output_bfd, sreloc, &outrel);
> +               {
> +                 if (is_pic && r_type != R_LARCH_NN)
> +                   {
> +                     /* Not to use ELFCLASSNN in string literal or it'll
> +                        puzzle gettext.  */
> +
> +                     /* xgettext:c-format  */
> +                     char *msg = bfd_asprintf (
> +                       _("reloc is unresolved and cannot be turned to "
> +                         "a runtime reloc in ELFCLASS%d"),
> +                       NN);
> +
> +                     /* loongarch_reloc_is_fatal will output
> +                        "R_LARCH_32" or "R_LARCH_64" for us.  */
> +                     fatal = loongarch_reloc_is_fatal (
> +                       info, input_bfd, input_section, rel, howto,
> +                       bfd_reloc_notsupported, is_undefweak, name, msg);
> +                   }
> +                 else
> +                   loongarch_elf_append_rela (output_bfd, sreloc,
> +                                              &outrel);
> +               }
>             }
>
>           relocation += rel->r_addend;
> diff --git a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
> index d833a89246e..70625fa8dfe 100644
> --- a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
> +++ b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
> @@ -139,6 +139,7 @@ if [istarget "loongarch64-*-*"] {
>      run_dump_test "reloc_le_with_shared"
>      run_dump_test "reloc_ler_with_shared"
>      run_dump_test "reloc_abs_with_shared"
> +    run_dump_test "r_larch_32_elf64"
>    }
>
>    if [check_pie_support] {
> diff --git a/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d
> new file mode 100644
> index 00000000000..df61f3a36c5
> --- /dev/null
> +++ b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d
> @@ -0,0 +1,4 @@
> +#name: R_LARCH_32 in ELFCLASS64
> +#source: r_larch_32_elf64.s
> +#ld: -shared -melf64loongarch
> +#error: R_LARCH_32 against `x':\nreloc is unresolved and cannot be turned to a runtime reloc in ELFCLASS64
> diff --git a/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s
> new file mode 100644
> index 00000000000..6649f2bce01
> --- /dev/null
> +++ b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s
> @@ -0,0 +1,3 @@
> +.data
> +x:
> +       .4byte x
> --
> 2.45.2
>
mengqinggang June 24, 2024, 8:45 a.m. UTC | #2 
在 2024/6/22 下午6:03, Xi Ruoyao 写道:
> We were converting R_LARCH_32 to R_LARCH_RELATIVE for ELFCLASS64:
>
>      $ cat t.s
>      .data
>      x:
>          .4byte x
>      $ as/as-new t.s -o t.o
>      $ ld/ld-new -shared t.s
>      $ objdump -R
>      a.out:     file format elf64-loongarch
>
>      DYNAMIC RELOCATION RECORDS
>      OFFSET           TYPE              VALUE
>      00000000000001a8 R_LARCH_RELATIVE  *ABS*+0x00000000000001a8
>
> But this is just wrong: at runtime the dynamic linker will run
> *(uintptr *)&x += load_address, causing an OOB write.


OOB write means R_LARCH_RELATIVE writes 8 bytes to 4 bytes?


>
> It does not make too much sense to support R_LARCH_32 in ELFCLASS64 or
> R_LARCH_64 in ELFCLASS32 when creating a DLL or PIE.  And, if we keep it
> as-is in the linked object, it'll be rejected by Glibc dynamic linker
> anyway.  So we can just reject it like x86_64 and RISC-V.
>
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
>   bfd/elfnn-loongarch.c                         | 23 ++++++++++++++++++-
>   .../ld-loongarch-elf/ld-loongarch-elf.exp     |  1 +
>   .../ld-loongarch-elf/r_larch_32_elf64.d       |  4 ++++
>   .../ld-loongarch-elf/r_larch_32_elf64.s       |  3 +++
>   4 files changed, 30 insertions(+), 1 deletion(-)
>   create mode 100644 ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d
>   create mode 100644 ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index b1720760475..3a55ac93e20 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -2861,7 +2861,28 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
>   	      /* No alloc space of func allocate_dynrelocs.  */
>   	      if (unresolved_reloc
>   		  && !(h && (h->is_weakalias || !h->dyn_relocs)))
> -		loongarch_elf_append_rela (output_bfd, sreloc, &outrel);
> +		{
> +		  if (is_pic && r_type != R_LARCH_NN)
> +		    {
> +		      /* Not to use ELFCLASSNN in string literal or it'll
> +			 puzzle gettext.  */
> +
> +		      /* xgettext:c-format  */
> +		      char *msg = bfd_asprintf (
> +			_("reloc is unresolved and cannot be turned to "
> +			  "a runtime reloc in ELFCLASS%d"),
> +			NN);
> +
> +		      /* loongarch_reloc_is_fatal will output
> +			 "R_LARCH_32" or "R_LARCH_64" for us.  */
> +		      fatal = loongarch_reloc_is_fatal (
> +			info, input_bfd, input_section, rel, howto,
> +			bfd_reloc_notsupported, is_undefweak, name, msg);
> +		    }
> +		  else
> +		    loongarch_elf_append_rela (output_bfd, sreloc,
> +					       &outrel);
> +		}
>   	    }
>   
>   	  relocation += rel->r_addend;
> diff --git a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
> index d833a89246e..70625fa8dfe 100644
> --- a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
> +++ b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
> @@ -139,6 +139,7 @@ if [istarget "loongarch64-*-*"] {
>       run_dump_test "reloc_le_with_shared"
>       run_dump_test "reloc_ler_with_shared"
>       run_dump_test "reloc_abs_with_shared"
> +    run_dump_test "r_larch_32_elf64"
>     }
>   
>     if [check_pie_support] {
> diff --git a/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d
> new file mode 100644
> index 00000000000..df61f3a36c5
> --- /dev/null
> +++ b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d
> @@ -0,0 +1,4 @@
> +#name: R_LARCH_32 in ELFCLASS64
> +#source: r_larch_32_elf64.s
> +#ld: -shared -melf64loongarch
> +#error: R_LARCH_32 against `x':\nreloc is unresolved and cannot be turned to a runtime reloc in ELFCLASS64
> diff --git a/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s
> new file mode 100644
> index 00000000000..6649f2bce01
> --- /dev/null
> +++ b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s
> @@ -0,0 +1,3 @@
> +.data
> +x:
> +	.4byte x
Xi Ruoyao June 24, 2024, 8:52 a.m. UTC | #3 
On Mon, 2024-06-24 at 16:45 +0800, mengqinggang wrote:
> > But this is just wrong: at runtime the dynamic linker will run
> > *(uintptr *)&x += load_address, causing an OOB write.
> 
> 
> OOB write means R_LARCH_RELATIVE writes 8 bytes to 4 bytes?

Yep.  If the next 4 bytes have some important data bad things happen.
mengqinggang June 25, 2024, 2:26 a.m. UTC | #4 
在 2024/6/22 下午6:03, Xi Ruoyao 写道:
> We were converting R_LARCH_32 to R_LARCH_RELATIVE for ELFCLASS64:
>
>      $ cat t.s
>      .data
>      x:
>          .4byte x
>      $ as/as-new t.s -o t.o
>      $ ld/ld-new -shared t.s


t.s -> t.o?


>      $ objdump -R
>      a.out:     file format elf64-loongarch
>
>      DYNAMIC RELOCATION RECORDS
>      OFFSET           TYPE              VALUE
>      00000000000001a8 R_LARCH_RELATIVE  *ABS*+0x00000000000001a8
>
> But this is just wrong: at runtime the dynamic linker will run
> *(uintptr *)&x += load_address, causing an OOB write.
>
> It does not make too much sense to support R_LARCH_32 in ELFCLASS64 or
> R_LARCH_64 in ELFCLASS32 when creating a DLL or PIE.  And, if we keep it
> as-is in the linked object, it'll be rejected by Glibc dynamic linker
> anyway.  So we can just reject it like x86_64 and RISC-V.
>
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
>   bfd/elfnn-loongarch.c                         | 23 ++++++++++++++++++-
>   .../ld-loongarch-elf/ld-loongarch-elf.exp     |  1 +
>   .../ld-loongarch-elf/r_larch_32_elf64.d       |  4 ++++
>   .../ld-loongarch-elf/r_larch_32_elf64.s       |  3 +++
>   4 files changed, 30 insertions(+), 1 deletion(-)
>   create mode 100644 ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d
>   create mode 100644 ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index b1720760475..3a55ac93e20 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -2861,7 +2861,28 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
>   	      /* No alloc space of func allocate_dynrelocs.  */
>   	      if (unresolved_reloc
>   		  && !(h && (h->is_weakalias || !h->dyn_relocs)))
> -		loongarch_elf_append_rela (output_bfd, sreloc, &outrel);
> +		{
> +		  if (is_pic && r_type != R_LARCH_NN)
> +		    {
> +		      /* Not to use ELFCLASSNN in string literal or it'll
> +			 puzzle gettext.  */
> +
> +		      /* xgettext:c-format  */
> +		      char *msg = bfd_asprintf (
> +			_("reloc is unresolved and cannot be turned to "
> +			  "a runtime reloc in ELFCLASS%d"),
> +			NN);
> +
> +		      /* loongarch_reloc_is_fatal will output
> +			 "R_LARCH_32" or "R_LARCH_64" for us.  */
> +		      fatal = loongarch_reloc_is_fatal (
> +			info, input_bfd, input_section, rel, howto,
> +			bfd_reloc_notsupported, is_undefweak, name, msg);
> +		    }
> +		  else
> +		    loongarch_elf_append_rela (output_bfd, sreloc,
> +					       &outrel);
> +		}
>   	    }
>   
>   	  relocation += rel->r_addend;
> diff --git a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
> index d833a89246e..70625fa8dfe 100644
> --- a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
> +++ b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
> @@ -139,6 +139,7 @@ if [istarget "loongarch64-*-*"] {
>       run_dump_test "reloc_le_with_shared"
>       run_dump_test "reloc_ler_with_shared"
>       run_dump_test "reloc_abs_with_shared"
> +    run_dump_test "r_larch_32_elf64"
>     }
>   
>     if [check_pie_support] {
> diff --git a/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d
> new file mode 100644
> index 00000000000..df61f3a36c5
> --- /dev/null
> +++ b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d
> @@ -0,0 +1,4 @@
> +#name: R_LARCH_32 in ELFCLASS64
> +#source: r_larch_32_elf64.s
> +#ld: -shared -melf64loongarch
> +#error: R_LARCH_32 against `x':\nreloc is unresolved and cannot be turned to a runtime reloc in ELFCLASS64
> diff --git a/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s
> new file mode 100644
> index 00000000000..6649f2bce01
> --- /dev/null
> +++ b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s
> @@ -0,0 +1,3 @@
> +.data
> +x:
> +	.4byte x
diff mbox series

Patch

diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index b1720760475..3a55ac93e20 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -2861,7 +2861,28 @@  loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
 	      /* No alloc space of func allocate_dynrelocs.  */
 	      if (unresolved_reloc
 		  && !(h && (h->is_weakalias || !h->dyn_relocs)))
-		loongarch_elf_append_rela (output_bfd, sreloc, &outrel);
+		{
+		  if (is_pic && r_type != R_LARCH_NN)
+		    {
+		      /* Not to use ELFCLASSNN in string literal or it'll
+			 puzzle gettext.  */
+
+		      /* xgettext:c-format  */
+		      char *msg = bfd_asprintf (
+			_("reloc is unresolved and cannot be turned to "
+			  "a runtime reloc in ELFCLASS%d"),
+			NN);
+
+		      /* loongarch_reloc_is_fatal will output
+			 "R_LARCH_32" or "R_LARCH_64" for us.  */
+		      fatal = loongarch_reloc_is_fatal (
+			info, input_bfd, input_section, rel, howto,
+			bfd_reloc_notsupported, is_undefweak, name, msg);
+		    }
+		  else
+		    loongarch_elf_append_rela (output_bfd, sreloc,
+					       &outrel);
+		}
 	    }
 
 	  relocation += rel->r_addend;
diff --git a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
index d833a89246e..70625fa8dfe 100644
--- a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
+++ b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
@@ -139,6 +139,7 @@  if [istarget "loongarch64-*-*"] {
     run_dump_test "reloc_le_with_shared"
     run_dump_test "reloc_ler_with_shared"
     run_dump_test "reloc_abs_with_shared"
+    run_dump_test "r_larch_32_elf64"
   }
 
   if [check_pie_support] {
diff --git a/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d
new file mode 100644
index 00000000000..df61f3a36c5
--- /dev/null
+++ b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d
@@ -0,0 +1,4 @@ 
+#name: R_LARCH_32 in ELFCLASS64
+#source: r_larch_32_elf64.s
+#ld: -shared -melf64loongarch
+#error: R_LARCH_32 against `x':\nreloc is unresolved and cannot be turned to a runtime reloc in ELFCLASS64
diff --git a/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s
new file mode 100644
index 00000000000..6649f2bce01
--- /dev/null
+++ b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s
@@ -0,0 +1,3 @@ 
+.data
+x:
+	.4byte x