This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] elf: Try not pointing empty PT_LOAD segment's offset past EOF


Hi Alan,

Alan Modra <amodra@gmail.com> ezt írta (időpont: 2019. dec. 11., Sze, 1:44):
>
> On Tue, Dec 10, 2019 at 09:44:28PM +0100, Bálint Réczey wrote:
> > Hi,
> >
> > Florian Weimer <fweimer@redhat.com> ezt írta (időpont: 2019. dec. 10.,
> > K, 14:23):
> > >
> > > * Alan Modra:
> > >
> > > > On Sat, Dec 07, 2019 at 05:22:13PM +0100, Bálint Réczey wrote:
> > > >> diff --git a/bfd/elf.c b/bfd/elf.c
> > > >> index 1aa2603ee8..e1a9a02eec 100644
> > > >> --- a/bfd/elf.c
> > > >> +++ b/bfd/elf.c
> > > >> @@ -5752,7 +5752,12 @@ assign_file_positions_for_load_sections (bfd *abfd,
> > > >>        || (p->p_type == PT_NOTE && bfd_get_format (abfd) == bfd_core))
> > > >>      {
> > > >>        if (!m->includes_filehdr && !m->includes_phdrs)
> > > >> -        p->p_offset = off;
> > > >> +        if (no_contents)
> > > >> +          /* Try avoiding pointing past the EOF with this empty segment's
> > > >> +             p_offset. */
> > > >> +          p->p_offset = p->p_offset % maxpagesize;
> > > >> +        else
> > > >> +          p->p_offset = off;
> > > >>        else
> > > >>          {
> > > >>            file_ptr adjust;
> > > >
> > > > How did you test this patch?  I suspect you are just leaving p_offset
> > > > at zero and therefore will cause failures on glibc systems.
> > >
> > > I think glibc requires ordering by increasing virtual address, per the
> > > specification.  That does not seem to change here.
> > >
> > > Testing on glibc is of course recommended, though.  It also needs to be
> > > tested with various kernel versions.
> >
> > For the record I've tested the rebuilt gzip binary on Ubuntu's
> > 5.0.0-37-generic and on 5.3.0-24-generic.
>
> "p->p_offset = p->p_offset % maxpagesize" is not the best way to write
> "p->p_offset = 0".  So I'd reject this patch for not doing as the
> submitter intended.

I'm sorry but my intention was keeping p_offset mod maxpagesize ==
p_vaddr mod maxpagesize and as I understand it is not guaranteed when
just doing p->p_offset = 0.
Keeping the modulo was suggested by you in bugzilla, and thanks for that.

> Using zero will almost certainly fail for some binaries.

Please tell how can I find/build such binaries, I believe changing
only the offset in the file keeping should not cause problem anywhere.
If zero indeed causes problems I propose doing:

if (p->p_offset > maxpagesize) p->p_offset -= maxpagesize;

, but I believe % maxpagesize is simpler and still works.

Cheers,
Balint

>
> --
> Alan Modra
> Australia Development Lab, IBM


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]