Re: [PING v2] [PATCH] Test for __mprotect failure in _dl_map_segments [BZ #20831]

[Zack Weinberg <zackw at panix dot com>]

On Mon, Mar 20, 2017 at 3:35 PM, Dmitry V. Levin <ldv@altlinux.org> wrote:
>
> I understand the patch is trivial, but anyway, there is a bug and it has
> to be fixed.
> If there are no comments, I'd push it rather than go on with these ping
> reposts.

We have a collective problem where nobody feels empowered to say "yes"
to patches.

Also, you found this bug by fault injection -- do you have reason to
believe that this mprotect call can actually fail? If so, under what
circumstances, and how bad are the consequences?

> +        {
> +          /* 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.  */
> +          int rc;
> +          rc = __mprotect ((caddr_t) (l->l_addr + c->mapend),
> +                           loadcmds[nloadcmds - 1].mapstart - c->mapend,
> +                           PROT_NONE);
> +          if (__glibc_unlikely (rc < 0))
> +            return DL_MAP_SEGMENTS_ERROR_MPROTECT;
> +        }

The variable 'rc' appears to be unnecessary.  Why not just

    if (__glibc_unlikely (__mprotect (...) < 0))
      return DL_MAP_SEGMENTS_ERROR_MPROTECT;

?