All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niklas Cassel <niklas.cassel@wdc.com>
To: Alexander Viro <viro@zeniv.linux.org.uk>,
	Eric Biederman <ebiederm@xmission.com>,
	Kees Cook <keescook@chromium.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>
Cc: Greg Ungerer <gerg@linux-m68k.org>,
	Mike Frysinger <vapier@gentoo.org>,
	Damien Le Moal <damien.lemoal@opensource.wdc.com>,
	Niklas Cassel <niklas.cassel@wdc.com>,
	stable@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-mm@kvack.org, linux-riscv@lists.infradead.org
Subject: [PATCH v2] binfmt_flat: do not stop relocating GOT entries prematurely on riscv
Date: Thu, 14 Apr 2022 11:10:18 +0200	[thread overview]
Message-ID: <20220414091018.896737-1-niklas.cassel@wdc.com> (raw)

bFLT binaries are usually created using elf2flt.

The linker script used by elf2flt has defined the .data section like the
following for the last 19 years:

.data : {
	_sdata = . ;
	__data_start = . ;
	data_start = . ;
	*(.got.plt)
	*(.got)
	FILL(0) ;
	. = ALIGN(0x20) ;
	LONG(-1)
	. = ALIGN(0x20) ;
	...
}

It places the .got.plt input section before the .got input section.
The same is true for the default linker script (ld --verbose) on most
architectures except x86/x86-64.

The binfmt_flat loader should relocate all GOT entries until it encounters
a -1 (the LONG(-1) in the linker script).

The problem is that the .got.plt input section starts with a GOTPLT header
(which has size 16 bytes on elf64-riscv and 8 bytes on elf32-riscv), where
the first word is set to -1. See the binutils implementation for riscv [1].

This causes the binfmt_flat loader to stop relocating GOT entries
prematurely and thus causes the application to crash when running.

Fix this by skipping the whole GOTPLT header, since the whole GOTPLT header
is reserved for the dynamic linker.

The GOTPLT header will only be skipped for bFLT binaries with flag
FLAT_FLAG_GOTPIC set. This flag is unconditionally set by elf2flt if the
supplied ELF binary has the symbol _GLOBAL_OFFSET_TABLE_ defined.
ELF binaries without a .got input section should thus remain unaffected.

Tested on RISC-V Canaan Kendryte K210 and RISC-V QEMU nommu_virt_defconfig.

[1] https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/elfnn-riscv.c;hb=binutils-2_38#l3275

Cc: <stable@vger.kernel.org>
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
Changes since v1:
-Incorporated review comments from Eric Biederman.

RISC-V elf2flt patches are still not merged, they can be found here:
https://github.com/floatious/elf2flt/tree/riscv

buildroot branch for k210 nommu (including this patch and elf2flt patches):
https://github.com/floatious/buildroot/tree/k210-v14

 fs/binfmt_flat.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 626898150011..e5e2a03b39c1 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -440,6 +440,30 @@ static void old_reloc(unsigned long rl)
 
 /****************************************************************************/
 
+static inline u32 __user *skip_got_header(u32 __user *rp)
+{
+	if (IS_ENABLED(CONFIG_RISCV)) {
+		/*
+		 * RISC-V has a 16 byte GOT PLT header for elf64-riscv
+		 * and 8 byte GOT PLT header for elf32-riscv.
+		 * Skip the whole GOT PLT header, since it is reserved
+		 * for the dynamic linker (ld.so).
+		 */
+		u32 rp_val0, rp_val1;
+
+		if (get_user(rp_val0, rp))
+			return rp;
+		if (get_user(rp_val1, rp + 1))
+			return rp;
+
+		if (rp_val0 == 0xffffffff && rp_val1 == 0xffffffff)
+			rp += 4;
+		else if (rp_val0 == 0xffffffff)
+			rp += 2;
+	}
+	return rp;
+}
+
 static int load_flat_file(struct linux_binprm *bprm,
 		struct lib_info *libinfo, int id, unsigned long *extra_stack)
 {
@@ -789,7 +813,8 @@ static int load_flat_file(struct linux_binprm *bprm,
 	 * image.
 	 */
 	if (flags & FLAT_FLAG_GOTPIC) {
-		for (rp = (u32 __user *)datapos; ; rp++) {
+		rp = skip_got_header((u32 * __user) datapos);
+		for (; ; rp++) {
 			u32 addr, rp_val;
 			if (get_user(rp_val, rp))
 				return -EFAULT;
-- 
2.35.1


WARNING: multiple messages have this Message-ID (diff)
From: Niklas Cassel <niklas.cassel@wdc.com>
To: Alexander Viro <viro@zeniv.linux.org.uk>,
	Eric Biederman <ebiederm@xmission.com>,
	Kees Cook <keescook@chromium.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>
Cc: Greg Ungerer <gerg@linux-m68k.org>,
	Mike Frysinger <vapier@gentoo.org>,
	Damien Le Moal <damien.lemoal@opensource.wdc.com>,
	Niklas Cassel <niklas.cassel@wdc.com>,
	stable@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-mm@kvack.org, linux-riscv@lists.infradead.org
Subject: [PATCH v2] binfmt_flat: do not stop relocating GOT entries prematurely on riscv
Date: Thu, 14 Apr 2022 11:10:18 +0200	[thread overview]
Message-ID: <20220414091018.896737-1-niklas.cassel@wdc.com> (raw)

bFLT binaries are usually created using elf2flt.

The linker script used by elf2flt has defined the .data section like the
following for the last 19 years:

.data : {
	_sdata = . ;
	__data_start = . ;
	data_start = . ;
	*(.got.plt)
	*(.got)
	FILL(0) ;
	. = ALIGN(0x20) ;
	LONG(-1)
	. = ALIGN(0x20) ;
	...
}

It places the .got.plt input section before the .got input section.
The same is true for the default linker script (ld --verbose) on most
architectures except x86/x86-64.

The binfmt_flat loader should relocate all GOT entries until it encounters
a -1 (the LONG(-1) in the linker script).

The problem is that the .got.plt input section starts with a GOTPLT header
(which has size 16 bytes on elf64-riscv and 8 bytes on elf32-riscv), where
the first word is set to -1. See the binutils implementation for riscv [1].

This causes the binfmt_flat loader to stop relocating GOT entries
prematurely and thus causes the application to crash when running.

Fix this by skipping the whole GOTPLT header, since the whole GOTPLT header
is reserved for the dynamic linker.

The GOTPLT header will only be skipped for bFLT binaries with flag
FLAT_FLAG_GOTPIC set. This flag is unconditionally set by elf2flt if the
supplied ELF binary has the symbol _GLOBAL_OFFSET_TABLE_ defined.
ELF binaries without a .got input section should thus remain unaffected.

Tested on RISC-V Canaan Kendryte K210 and RISC-V QEMU nommu_virt_defconfig.

[1] https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/elfnn-riscv.c;hb=binutils-2_38#l3275

Cc: <stable@vger.kernel.org>
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
Changes since v1:
-Incorporated review comments from Eric Biederman.

RISC-V elf2flt patches are still not merged, they can be found here:
https://github.com/floatious/elf2flt/tree/riscv

buildroot branch for k210 nommu (including this patch and elf2flt patches):
https://github.com/floatious/buildroot/tree/k210-v14

 fs/binfmt_flat.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 626898150011..e5e2a03b39c1 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -440,6 +440,30 @@ static void old_reloc(unsigned long rl)
 
 /****************************************************************************/
 
+static inline u32 __user *skip_got_header(u32 __user *rp)
+{
+	if (IS_ENABLED(CONFIG_RISCV)) {
+		/*
+		 * RISC-V has a 16 byte GOT PLT header for elf64-riscv
+		 * and 8 byte GOT PLT header for elf32-riscv.
+		 * Skip the whole GOT PLT header, since it is reserved
+		 * for the dynamic linker (ld.so).
+		 */
+		u32 rp_val0, rp_val1;
+
+		if (get_user(rp_val0, rp))
+			return rp;
+		if (get_user(rp_val1, rp + 1))
+			return rp;
+
+		if (rp_val0 == 0xffffffff && rp_val1 == 0xffffffff)
+			rp += 4;
+		else if (rp_val0 == 0xffffffff)
+			rp += 2;
+	}
+	return rp;
+}
+
 static int load_flat_file(struct linux_binprm *bprm,
 		struct lib_info *libinfo, int id, unsigned long *extra_stack)
 {
@@ -789,7 +813,8 @@ static int load_flat_file(struct linux_binprm *bprm,
 	 * image.
 	 */
 	if (flags & FLAT_FLAG_GOTPIC) {
-		for (rp = (u32 __user *)datapos; ; rp++) {
+		rp = skip_got_header((u32 * __user) datapos);
+		for (; ; rp++) {
 			u32 addr, rp_val;
 			if (get_user(rp_val, rp))
 				return -EFAULT;
-- 
2.35.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

             reply	other threads:[~2022-04-14  9:11 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-14  9:10 Niklas Cassel [this message]
2022-04-14  9:10 ` [PATCH v2] binfmt_flat: do not stop relocating GOT entries prematurely on riscv Niklas Cassel
2022-04-14 23:05 ` Kees Cook
2022-04-14 23:05   ` Kees Cook
2022-04-15  1:24   ` Niklas Cassel
2022-04-15  1:24     ` Niklas Cassel
2022-04-14 23:27 ` Kees Cook
2022-04-14 23:27   ` Kees Cook
2022-04-14 23:41   ` Damien Le Moal
2022-04-14 23:41     ` Damien Le Moal
2022-04-15  1:26   ` Konstantin Ryabitsev
2022-04-15  1:26     ` Konstantin Ryabitsev
2022-04-16  0:14     ` Kees Cook
2022-04-16  0:14       ` Kees Cook
2022-04-14 23:51 ` Damien Le Moal
2022-04-14 23:51   ` Damien Le Moal
2022-04-15  0:30   ` Niklas Cassel
2022-04-15  0:30     ` Niklas Cassel
2022-04-15  0:56     ` Damien Le Moal
2022-04-15  0:56       ` Damien Le Moal
2022-04-15  1:08       ` Niklas Cassel
2022-04-15  1:08         ` Niklas Cassel
2022-04-15  1:13         ` Damien Le Moal
2022-04-15  1:13           ` Damien Le Moal
2022-04-15  2:11           ` Niklas Cassel
2022-04-15  2:11             ` Niklas Cassel
2022-04-15  2:14             ` Damien Le Moal
2022-04-15  2:14               ` Damien Le Moal
2022-04-20  4:04     ` Greg Ungerer
2022-04-20  4:04       ` Greg Ungerer
2022-04-20 14:58       ` [PATCH] binfmt_flat: Remove shared library support Eric W. Biederman
2022-04-20 14:58         ` Eric W. Biederman
2022-04-20 14:58         ` Eric W. Biederman
2022-04-20 14:58         ` Eric W. Biederman
2022-04-20 16:17         ` Palmer Dabbelt
2022-04-20 16:17           ` Palmer Dabbelt
2022-04-20 16:17           ` Palmer Dabbelt
2022-04-20 16:59           ` Rich Felker
2022-04-20 16:59             ` Rich Felker
2022-04-20 16:59             ` Rich Felker
2022-04-20 17:47             ` Kees Cook
2022-04-20 17:47               ` Kees Cook
2022-04-20 17:47               ` Kees Cook
2022-04-20 20:04               ` Arnd Bergmann
2022-04-20 20:04                 ` Arnd Bergmann
2022-04-20 20:04                 ` Arnd Bergmann
2022-04-20 20:23                 ` Rich Felker
2022-04-20 20:23                   ` Rich Felker
2022-04-20 20:23                   ` Rich Felker
2022-04-20 23:00                   ` Damien Le Moal
2022-04-20 23:00                     ` Damien Le Moal
2022-04-20 23:00                     ` Damien Le Moal
2022-04-25  3:38               ` Rob Landley
2022-04-25  3:38                 ` Rob Landley
2022-04-25  3:38                 ` Rob Landley
2022-04-25  7:40                 ` Greg Ungerer
2022-04-25  7:40                   ` Greg Ungerer
2022-04-25  7:40                   ` Greg Ungerer
2022-04-20 23:36         ` Damien Le Moal
2022-04-20 23:36           ` Damien Le Moal
2022-04-20 23:36           ` Damien Le Moal
2022-04-20 23:53         ` Greg Ungerer
2022-04-20 23:53           ` Greg Ungerer
2022-04-20 23:53           ` Greg Ungerer
2022-04-21  6:52           ` Geert Uytterhoeven
2022-04-21  6:52             ` Geert Uytterhoeven
2022-04-21  6:52             ` Geert Uytterhoeven
2022-04-21  7:12             ` Arnd Bergmann
2022-04-21  7:12               ` Arnd Bergmann
2022-04-21  7:12               ` Arnd Bergmann
2022-04-22 10:26               ` Vladimir Murzin
2022-04-22 10:26                 ` Vladimir Murzin
2022-04-22 10:26                 ` Vladimir Murzin
2022-04-22 15:18               ` Patrice CHOTARD
2022-04-21 12:43             ` Rich Felker
2022-04-21 12:43               ` Rich Felker
2022-04-21 12:43               ` Rich Felker
2022-04-25  3:50               ` Rob Landley
2022-04-25  3:50                 ` Rob Landley
2022-04-25  3:50                 ` Rob Landley
2022-04-21  0:05         ` (subset) " Kees Cook
2022-04-21  0:05           ` Kees Cook
2022-04-21  0:05           ` Kees Cook
2022-04-16  4:25 ` [PATCH v2] binfmt_flat: do not stop relocating GOT entries prematurely on riscv Kees Cook
2022-04-16  4:25   ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220414091018.896737-1-niklas.cassel@wdc.com \
    --to=niklas.cassel@wdc.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=damien.lemoal@opensource.wdc.com \
    --cc=ebiederm@xmission.com \
    --cc=gerg@linux-m68k.org \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=stable@vger.kernel.org \
    --cc=vapier@gentoo.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.