Re: [PATCH] elf: Remove the 1-page gap before the RELRO segment

["H.J. Lu" <hjl.tools@gmail.com>]

On Thu, Jan 13, 2022 at 4:52 AM Alan Modra <amodra@gmail.com> wrote:
>
> On Mon, Jan 10, 2022 at 06:12:41PM -0800, H.J. Lu via Binutils wrote:
> > The existing RELRO scheme may leave a 1-page gap before the RELRO segment
> > and align the end of the RELRO segment to the page size:
> >
> >   [18] .eh_frame    PROGBITS    408fa0 008fa0 005e80 00   A  0   0  8
> >   [19] .init_array  INIT_ARRAY  410de0 00fde0 000008 08  WA  0   0  8
> >   [20] .fini_array  FINI_ARRAY  410de8 00fde8 000008 08  WA  0   0  8
> >   [21] .dynamic     DYNAMIC     410df0 00fdf0 000200 10  WA  7   0  8
> >   [22] .got         PROGBITS    410ff0 00fff0 000010 08  WA  0   0  8
> >   [23] .got.plt     PROGBITS    411000 010000 000048 08  WA  0   0  8
>
> Do you know what is going wrong with the relro section layout for this
> to occur?
>
> In this particular case, the end of the read-only segment is at
> 0x408fa0 + 0x5e80 = 0x40ee20.  My guess is that layout of the
> following rw sections starts on the next page plus current offset
> within page, the standard choice to minimise disk pages.  ie. We start
> at 0x40fe20.  Then discover that this puts .got.plt at 0x40fe20 + 8 +
> 8 + 0x200 + 0x10 = 0x40f040.  However, we want this to be on a page
> boundary so that the relro segment ends on a page boundary.  So we
> bump 0x40f040 up to 0x411000 and calculate backwards from there to
> arrive at .init_array with a vma of 0x410de0.  Resulting in the
> 0x40f000 page being unused.
>
> If instead we start relro layout on the next page, we'd start laying
> out at 0x40f000 rather than 0x40fe20.  I think that would be the

But if the prior ro section size is greater than one page, we want
to subtract 1 page:

+  /* If the preceding section size is greater than the maximum
+      page size, subtract the maximum page size.  Otherwise,
+      align the RELRO segment to the maximum page size.  */
+   if (prev_sec->size > seg->maxpagesize)
+     {
+       desired_end -= seg->maxpagesize;
+       relro_end -= seg->maxpagesize;
+     }
+   else
+     {
+       desired_end &= ~(seg->maxpagesize - 1);
+       relro_end &= ~(seg->maxpagesize - 1);
+     }
+   }

> correct thing to do rather than fixing up afterwards as your patch
> does.
>

I am checking in my patch as Nick has approved it. There is a
possibility that one 1-page gap may be needed to maintain
the section alignment.  My patch checks it and includes many
testcase adjustments to remove one 1-page gap.   Can you
update the RELRO segment algorithm to make my fixup
unnecessary?

Thanks.

-- 
H.J.