Message ID | 20220215145711.3331849-1-hjl.tools@gmail.com |
---|---|
State | New |
Headers | show |
Series | [v2] elf: Check invalid hole in PT_LOAD segments [BZ #28838] | expand |
* H. J. Lu: > Changes in v2: > > 1. Update commit log. > > commit 163f625cf9becbb82dfec63a29e566324129c0cd > Author: H.J. Lu <hjl.tools@gmail.com> > Date: Tue Dec 21 12:35:47 2021 -0800 > > elf: Remove excessive p_align check on PT_LOAD segments [BZ #28688] > > removed the p_align check against the page size. It caused the loader > error or crash on elf/tst-p_align3 when loading elf/tst-p_alignmod3.so, > which has the invalid p_align in PT_LOAD segments, added by > > commit d8d94863ef125a392b929732b37e07dc927fbcd1 > Author: H.J. Lu <hjl.tools@gmail.com> > Date: Tue Dec 21 13:42:28 2021 -0800 > > The loader failure caused by a negative length passed to __mprotect is > random, depending on architecture and toolchain. Update _dl_map_segments > to detect invalid holes. This fixes BZ #28838. > --- > elf/dl-map-segments.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/elf/dl-map-segments.h b/elf/dl-map-segments.h > index b3513e7909..024175b2d5 100644 > --- a/elf/dl-map-segments.h > +++ b/elf/dl-map-segments.h > @@ -112,6 +112,9 @@ _dl_map_segments (struct link_map *l, int fd, > unallocated. Then jump into the normal segment-mapping loop to > handle the portion of the segment past the end of the file > mapping. */ > + if (__glibc_unlikely (loadcmds[nloadcmds - 1].mapstart < > + c->mapend)) > + return N_("ELF load command address/offset not page-aligned"); > if (__glibc_unlikely > (__mprotect ((caddr_t) (l->l_addr + c->mapend), > loadcmds[nloadcmds - 1].mapstart - c->mapend, This looks okay to me. Avoiding the invalid mprotect call is a good idea. Thanks, Florian
On Mon, Feb 21, 2022 at 6:32 AM Florian Weimer <fweimer@redhat.com> wrote: > > * H. J. Lu: > > > Changes in v2: > > > > 1. Update commit log. > > > > commit 163f625cf9becbb82dfec63a29e566324129c0cd > > Author: H.J. Lu <hjl.tools@gmail.com> > > Date: Tue Dec 21 12:35:47 2021 -0800 > > > > elf: Remove excessive p_align check on PT_LOAD segments [BZ #28688] > > > > removed the p_align check against the page size. It caused the loader > > error or crash on elf/tst-p_align3 when loading elf/tst-p_alignmod3.so, > > which has the invalid p_align in PT_LOAD segments, added by > > > > commit d8d94863ef125a392b929732b37e07dc927fbcd1 > > Author: H.J. Lu <hjl.tools@gmail.com> > > Date: Tue Dec 21 13:42:28 2021 -0800 > > > > The loader failure caused by a negative length passed to __mprotect is > > random, depending on architecture and toolchain. Update _dl_map_segments > > to detect invalid holes. This fixes BZ #28838. > > --- > > elf/dl-map-segments.h | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/elf/dl-map-segments.h b/elf/dl-map-segments.h > > index b3513e7909..024175b2d5 100644 > > --- a/elf/dl-map-segments.h > > +++ b/elf/dl-map-segments.h > > @@ -112,6 +112,9 @@ _dl_map_segments (struct link_map *l, int fd, > > unallocated. Then jump into the normal segment-mapping loop to > > handle the portion of the segment past the end of the file > > mapping. */ > > + if (__glibc_unlikely (loadcmds[nloadcmds - 1].mapstart < > > + c->mapend)) > > + return N_("ELF load command address/offset not page-aligned"); > > if (__glibc_unlikely > > (__mprotect ((caddr_t) (l->l_addr + c->mapend), > > loadcmds[nloadcmds - 1].mapstart - c->mapend, > > This looks okay to me. Avoiding the invalid mprotect call is a good > idea. > > I am checking it in and backporting it to 2.35 branch. Thanks.
diff --git a/elf/dl-map-segments.h b/elf/dl-map-segments.h index b3513e7909..024175b2d5 100644 --- a/elf/dl-map-segments.h +++ b/elf/dl-map-segments.h @@ -112,6 +112,9 @@ _dl_map_segments (struct link_map *l, int fd, unallocated. Then jump into the normal segment-mapping loop to handle the portion of the segment past the end of the file mapping. */ + if (__glibc_unlikely (loadcmds[nloadcmds - 1].mapstart < + c->mapend)) + return N_("ELF load command address/offset not page-aligned"); if (__glibc_unlikely (__mprotect ((caddr_t) (l->l_addr + c->mapend), loadcmds[nloadcmds - 1].mapstart - c->mapend,