From: Niklas Cassel <Niklas.Cassel@wdc.com> To: Damien Le Moal <damien.lemoal@opensource.wdc.com> Cc: 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>, Greg Ungerer <gerg@linux-m68k.org>, Mike Frysinger <vapier@gentoo.org>, "stable@vger.kernel.org" <stable@vger.kernel.org>, "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>, "linux-mm@kvack.org" <linux-mm@kvack.org>, "linux-riscv@lists.infradead.org" <linux-riscv@lists.infradead.org> Subject: Re: [PATCH v2] binfmt_flat: do not stop relocating GOT entries prematurely on riscv Date: Fri, 15 Apr 2022 00:30:56 +0000 [thread overview] Message-ID: <Yli8voX7hw3EZ7E/@x1-carbon> (raw) In-Reply-To: <f379cb56-6ff5-f256-d5f2-3718a47e976d@opensource.wdc.com> On Fri, Apr 15, 2022 at 08:51:27AM +0900, Damien Le Moal wrote: > On 4/14/22 18:10, Niklas Cassel wrote: (snip) > This looks good to me. But thinking more about it, do we really need to > check what the content of the header is ? Why not simply replace this > entire hunk with: > > return rp + sizeof(unsigned long) * 2; > > to ignore the 16B (or 8B for 32-bits arch) header regardless of what the > header word values are ? Are there any case where the header is *not* > present ? Considering that I haven't been able to find any real specification that describes the bFLT format. (No, the elf2flt source is no specification.) This whole format seems kind of fragile. I realize that checking the first one or two entries after data start is not the most robust thing, but I still prefer it over skipping blindly. Especially considering that only m68k seems to support shared libraries with bFLT. So even while this header is reserved for ld.so, it will most likely only be used on m68k bFLT binaries.. so perhaps elf2flt some day decides to strip away this header on all bFLT binaries except for m68k? bFLT seems to currently be at version 4, perhaps such a change would require a version bump.. Or not? (Now, if there only was a spec.. :P) Kind regards, Niklas > > > + } > > + 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; > > Regardless of the above nit, feel free to add: > > Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> > > > -- > Damien Le Moal > Western Digital Research
WARNING: multiple messages have this Message-ID (diff)
From: Niklas Cassel <Niklas.Cassel@wdc.com> To: Damien Le Moal <damien.lemoal@opensource.wdc.com> Cc: 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>, Greg Ungerer <gerg@linux-m68k.org>, Mike Frysinger <vapier@gentoo.org>, "stable@vger.kernel.org" <stable@vger.kernel.org>, "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>, "linux-mm@kvack.org" <linux-mm@kvack.org>, "linux-riscv@lists.infradead.org" <linux-riscv@lists.infradead.org> Subject: Re: [PATCH v2] binfmt_flat: do not stop relocating GOT entries prematurely on riscv Date: Fri, 15 Apr 2022 00:30:56 +0000 [thread overview] Message-ID: <Yli8voX7hw3EZ7E/@x1-carbon> (raw) In-Reply-To: <f379cb56-6ff5-f256-d5f2-3718a47e976d@opensource.wdc.com> On Fri, Apr 15, 2022 at 08:51:27AM +0900, Damien Le Moal wrote: > On 4/14/22 18:10, Niklas Cassel wrote: (snip) > This looks good to me. But thinking more about it, do we really need to > check what the content of the header is ? Why not simply replace this > entire hunk with: > > return rp + sizeof(unsigned long) * 2; > > to ignore the 16B (or 8B for 32-bits arch) header regardless of what the > header word values are ? Are there any case where the header is *not* > present ? Considering that I haven't been able to find any real specification that describes the bFLT format. (No, the elf2flt source is no specification.) This whole format seems kind of fragile. I realize that checking the first one or two entries after data start is not the most robust thing, but I still prefer it over skipping blindly. Especially considering that only m68k seems to support shared libraries with bFLT. So even while this header is reserved for ld.so, it will most likely only be used on m68k bFLT binaries.. so perhaps elf2flt some day decides to strip away this header on all bFLT binaries except for m68k? bFLT seems to currently be at version 4, perhaps such a change would require a version bump.. Or not? (Now, if there only was a spec.. :P) Kind regards, Niklas > > > + } > > + 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; > > Regardless of the above nit, feel free to add: > > Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> > > > -- > Damien Le Moal > Western Digital Research _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2022-04-15 0:31 UTC|newest] Thread overview: 85+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-04-14 9:10 [PATCH v2] binfmt_flat: do not stop relocating GOT entries prematurely on riscv Niklas Cassel 2022-04-14 9:10 ` 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 [this message] 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=Yli8voX7hw3EZ7E/@x1-carbon \ --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: linkBe 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.