Reading through exec_elf.c, I just noticed the uninitialized bdiff variable myself, and remembered this thread.
Tangentially, the code for worrying about zero-filling the last page is overzealous. We only need to zero-fill the page if memsz > filesz (accounting for alignment and page boundaries). In the common case (at least on amd64), we always have either memsz==filesz (most segments) or filesz==0 (segment for .bss), so we shouldn't need zero-filling. And actually, I think the logic for only doing zero-fill for writable segments is wrong. It looks to me like the ELF gABI specifies zero-fill semantics for memsz > filesz even for read-only segments. On Sat, Oct 5, 2013 at 4:27 PM, Philip Guenther <[email protected]> wrote: > On Thu, 15 Aug 2013, Maxime Villard wrote: >> Hum hum, after investigating a bit, and from what I understood - if I'm >> not mistaken, I think it would make sense if bdiff was set to >> >> bdiff = ph->p_vaddr - trunc_page(ph->p_vaddr); > > Yep. > >> Since we are supposed to align only on 2^n boundaries, if we get 0 or 1 >> we do not align on p_align. But p_vaddr still has to be aligned on >> PAGE_MASK (with trunc_page()); I read somewhere that ELF loaders are >> smart enough to adjust the address when it does not exactly fit a page >> boundary. So bdiff should be the difference with the original p_vaddr. >> Actually, bdiff is already set to this value in the other conditions. >> >> There's another problem, 'base' should also be initialized here. I would >> say that is should be set to the truncated p_vaddr plus the address at >> which we want to load: >> >> base = *addr + trunc_page(ph->p_vaddr); >> >> but I'm not sure at all. > > By the logic of the "ph->p_align > 1" case, it should be > base = *addr + trunc_page(ph->p_vaddr) - ph->p_vaddr; > > (I think the only reason the 'if' is necessary is that p_align==0 must be > treated the same as p_align==1. The ELF_TRUNC() macro handles an > alignment of 1 correctly, but it'll barf on 0.) > > > Philip Guenther >
