pk: mmap based on `p_filesz` fails when it is 0 (zero)

[TiagoTeixeira-synthara]

Hello there,

I have a RISC-V ELF binary (hello.elf) that I'm trying to run on spike pk.

Running it fails with couldn't open ELF program: hello.elf!.

After inspecting the source code and the binary, it appears that the binary has one program header where the FileSiz/p_filesz is 0 (zero).

(Partial output from readelf)

Program Headers:
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD           0x001000 0x00002000 0x00002000 0x001c8 0x001c8 R E 0x1000
  LOAD           0x002000 0x00003000 0x00003000 0x00000 0x00800 RW  0x1000

From the section headers, it seems that that memory area is for the stack:

(Partial output from readelf)

  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
...
  [ 6] .stack            NOBITS          00003000 002000 000800 00  WA  0   0  1

---

From the source code, pk/elf.c at function load_elf (referencing latest commit I'm at) [https://github.com/riscv-software-src/riscv-pk/blob/9637e60b96b21a7f85a85bf033b87f64fb823b6c/pk/elf.c#L85]()

(source)

      uintptr_t prepad = ph[i].p_vaddr % RISCV_PGSIZE;
      uintptr_t vaddr = ph[i].p_vaddr + bias;
      if (vaddr + ph[i].p_memsz > info->brk_min)
        info->brk_min = vaddr + ph[i].p_memsz;
      int flags2 = flags | (prepad ? MAP_POPULATE : 0);
      int prot = get_prot(ph[i].p_flags);
      if (__do_mmap(vaddr - prepad, ph[i].p_filesz + prepad, prot | PROT_WRITE, flags2, file, ph[i].p_offset - prepad) != vaddr - prepad)
        goto fail;

The length passed to the __do_mmap function is 0 (zero), having the __do_mmap function failing at [https://github.com/riscv-software-src/riscv-pk/blob/9637e60b96b21a7f85a85bf033b87f64fb823b6c/pk/mmap.c#L366]()

From the previous line:

  size_t npage = (length-1)/RISCV_PGSIZE+1;

The (length-1) may give the max value for size_t type giving npage a false big value, which would be bigger than the number of free pages

Modifications to the code for debugging purposes did show that it was indeed failing there.

---

As a suggestion on how to fix this, should the loader handle the cases where p_filesz is zero? Possibly just skipping over that memory mapping. In general, if p_memsz is zero, then it shouldn't even have the need to be loaded?

And should the mprotect call protect all of p_memsz memory instead of just p_filesz?

Thanks for the support

[aswaterman]

Evidently mmap is supposed to fail for size of 0 (though I'll admit getting that correct was by sheer luck). So the fix is to not call mmap in this case. Fixed in c917315bf7a91e897f6464c62bdda01f9635536a

I agree re: mprotect, too. Fixed in 6b5c8dbb6f40eacbce5b698e957630e56ec30879

[TiagoTeixeira-synthara]

Hey, thanks for the follow up, Looks good thanks for the fix