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 4:27 PM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> On Mon, Mar 20, 2017 at 03:58:22PM -0400, Zack Weinberg wrote:
>> 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?
>
> Every mprotect call that increases memory fragmentation can legitimately
> fail with ENOMEM, the fault injection technique is just a very easy way
> to reproduce the error.

OK, I'll accept that as sufficient reason to go forward with the patch.

>> The variable 'rc' appears to be unnecessary.  Why not just
>>
>>     if (__glibc_unlikely (__mprotect (...) < 0))
>>       return DL_MAP_SEGMENTS_ERROR_MPROTECT;
>>
>> ?
>
> I want to keep the code readable.  If I did this, the line would get
> too long and I'd have to cut
> loadcmds[nloadcmds - 1].mapstart - c->mapend
> into pieces making it harder to comprehend.

You could do it like this:

        {
          /* 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;
        }

with the arguments to mprotect not any further rightward.

zw