Re: [PATCH v2] binfmt_flat: do not stop relocating GOT entries prematurely on riscv

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 15/4/22 10:30, Niklas Cassel wrote:
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?

FWIW there has been talk for a couple of years now to remove the
shared library support for m68k. It doesn't get used - probably not
for a very long time now. And most likely doesn't even work anymore.

Regards
Greg



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@xxxxxxxxxxxxxxxxxx>


--
Damien Le Moal
Western Digital Research



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux