Well, at the time I didn't really know these stuff - this bug was found by my home-made code scanner, in an area I was not particularly familiar with. However, I did try to help as far as I could, but despite my pings, it has remained unfixed so far, and 5.5 was released with no care taken about that.
I vaguely remember that I later ended up with a working patch, but apparently I've lost it since. This bug crashed binaries, and I had told myself it was annoying enough to be worth an Erratum. Anyway, it ain't that bad, and I'm glad to see people waking up one year later. (it won't be necessary to cc' me again for this issue) Regards, Maxime Le 17/06/2014 00:29, Matthew Dempsky a écrit : > > 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 <guent...@gmail.com> 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 >> > >
