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

[Dmitry V. Levin]

On Mon, Mar 20, 2017 at 03:58:22PM -0400, Zack Weinberg wrote:
> 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.

Yes, we have this problem, unfortunately.

> 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.

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

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.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20170320/19b61817/attachment.sig>