[v2] Check for __mprotect failure in _dl_map_segments [BZ #20831]
Commit Message
* elf/dl-map-segments.h (_dl_map_segments): Check for failure
of __mprotect to change protection on the excess portion
to disallow all access.
---
v2: change formatting of __glibc_unlikely as suggested by Zack Weinberg
---
ChangeLog | 7 +++++++
elf/dl-map-segments.h | 20 ++++++++++++--------
2 files changed, 19 insertions(+), 8 deletions(-)
Comments
On Tue, Mar 21, 2017 at 05:20:28PM +0300, Dmitry V. Levin wrote:
> * elf/dl-map-segments.h (_dl_map_segments): Check for failure
> of __mprotect to change protection on the excess portion
> to disallow all access.
> ---
> v2: change formatting of __glibc_unlikely as suggested by Zack Weinberg
> ---
> ChangeLog | 7 +++++++
> elf/dl-map-segments.h | 20 ++++++++++++--------
> 2 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/elf/dl-map-segments.h b/elf/dl-map-segments.h
> index 31d6861..d36f9bd 100644
> --- a/elf/dl-map-segments.h
> +++ b/elf/dl-map-segments.h
> @@ -64,14 +64,18 @@ _dl_map_segments (struct link_map *l, int fd,
> l->l_addr = l->l_map_start - c->mapstart;
>
> if (has_holes)
> - /* Change protection on the excess portion to disallow all access;
> - the portions we do not remap later will be inaccessible as if
> - unallocated. Then jump into the normal segment-mapping loop to
> - handle the portion of the segment past the end of the file
> - mapping. */
> - __mprotect ((caddr_t) (l->l_addr + c->mapend),
> - loadcmds[nloadcmds - 1].mapstart - c->mapend,
> - PROT_NONE);
> + {
> + /* Change protection on the excess portion to disallow all access;
> + the portions we do not remap later will be inaccessible as if
> + 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
> + (__mprotect ((caddr_t) (l->l_addr + c->mapend),
> + loadcmds[nloadcmds - 1].mapstart - c->mapend,
> + PROT_NONE) < 0))
> + return DL_MAP_SEGMENTS_ERROR_MPROTECT;
> + }
>
> l->l_contiguous = 1;
>
There have been no more comments so I've pushed this fix.
@@ -64,14 +64,18 @@ _dl_map_segments (struct link_map *l, int fd,
l->l_addr = l->l_map_start - c->mapstart;
if (has_holes)
- /* Change protection on the excess portion to disallow all access;
- the portions we do not remap later will be inaccessible as if
- unallocated. Then jump into the normal segment-mapping loop to
- handle the portion of the segment past the end of the file
- mapping. */
- __mprotect ((caddr_t) (l->l_addr + c->mapend),
- loadcmds[nloadcmds - 1].mapstart - c->mapend,
- PROT_NONE);
+ {
+ /* Change protection on the excess portion to disallow all access;
+ the portions we do not remap later will be inaccessible as if
+ 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
+ (__mprotect ((caddr_t) (l->l_addr + c->mapend),
+ loadcmds[nloadcmds - 1].mapstart - c->mapend,
+ PROT_NONE) < 0))
+ return DL_MAP_SEGMENTS_ERROR_MPROTECT;
+ }
l->l_contiguous = 1;