From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102f.google.com (mail-pj1-x102f.google.com [IPv6:2607:f8b0:4864:20::102f]) by sourceware.org (Postfix) with ESMTPS id B29653948815 for ; Mon, 31 Jan 2022 16:00:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B29653948815 Received: by mail-pj1-x102f.google.com with SMTP id qe6-20020a17090b4f8600b001b7aaad65b9so8147604pjb.2 for ; Mon, 31 Jan 2022 08:00:25 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=IB+kCFsKdJlHKGbLDbqpa8trUIEl9ntQAQsThZ/Nz2c=; b=J+ItE0ZFk9+uJl/hsSNwOz+5ejjOcn0eWSysLj+UtmonKdeBKlmHPBVp//xDjQFO4T ffsC2FAl4H5zCQQQDthOyzPvxsIRvEt7wZUN3LOOFJMQn3BLG6rxhPT0ZMlNu1wuqRnq Q7mtTqj+XycNjZ5fpO6D31xxaPboG3o55iGY+KJ1AtUi257tzKajyRLkjAyQ0orUxK9u 65yHZQR8hyT+Ol8k1stnmPZ/VutwnSR4BHzhV/t6X3f9NUPMND1HvrrJ8+aAA2BV2BEa FAU1gRZp4XlQ2HSaV5x6x3t1gW8aRS7Eb0QhOvUOt48WV2EPkSCiIzfxrXxOjdWMDS3I sNmQ== X-Gm-Message-State: AOAM530gI6ScweK9y60Rg7g2nDL5e5mMO4CXOvOTky1mgA5pKbeCN/PX ld9OLNgsEtyPDVwSSU0u3BzktkKoDQVfzZ3ZHqVwapXc6mg= X-Google-Smtp-Source: ABdhPJzTDMBn+/y/WObDCoYtopn7kv6P6S0Tv6aaoYBn16B0cewWb1fw/YndmIaCuXzn9ULYkoK+V5s3BFcjRpCej58= X-Received: by 2002:a17:902:a708:: with SMTP id w8mr21635495plq.101.1643644824816; Mon, 31 Jan 2022 08:00:24 -0800 (PST) MIME-Version: 1.0 References: <20220131152452.1061323-1-hjl.tools@gmail.com> <877daf9anu.fsf@oldenburg.str.redhat.com> In-Reply-To: <877daf9anu.fsf@oldenburg.str.redhat.com> From: "H.J. Lu" Date: Mon, 31 Jan 2022 07:59:48 -0800 Message-ID: Subject: Re: [PATCH] elf: Check invalid hole in PT_LOAD segments [BZ #28838] To: Florian Weimer Cc: GNU C Library , "Carlos O'Donell" , Michael Hudson-Doyle Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3027.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 31 Jan 2022 16:00:31 -0000 On Mon, Jan 31, 2022 at 7:40 AM Florian Weimer wrote: > > * H. J. Lu: > > > commit 163f625cf9becbb82dfec63a29e566324129c0cd > > Author: H.J. Lu > > 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 > > crash in shared objects with the invalid p_align. Update _dl_map_segments > > to detect invalid holes. This fixes BZ #28838. > > Commit message should reference commit ID and mention the failing > test/module name. > > > --- > > 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 172692b120..fd24cf5d01 100644 > > --- a/elf/dl-map-segments.h > > +++ b/elf/dl-map-segments.h > > @@ -113,6 +113,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, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ If loadcmds[nloadcmds - 1].mapstart < c->mapend, the length argument passed to __mprotect is a huge number and invalid. > > This seems to be fairly risky because I don't think that so far, we > enforce increasing LOAD segment addresses (although required by te EHF > specification). > > Given > > LDFLAGS-tst-p_alignmod3.so += -Wl,-z,max-page-size=0x100,-z,common-page-size=0x100 > > and RELRO construction for that > > .../elf/tst-p_alignmod3.so: cannot change memory protections > > seems to be a valid failure string for this test. However, worst case, > there could be a different kind of failure, if the RELRO mprotect start > is page-aligned by chance, and the kernel rounds up the end address to a > page boundary. The RELRO protection then covers more than what the link > editor expected, and this can cause crashes later on. But this isn't > something we can detect easily, I think. > > Thanks, > Florian > -- H.J.