From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x529.google.com (mail-pg1-x529.google.com [IPv6:2607:f8b0:4864:20::529]) by sourceware.org (Postfix) with ESMTPS id CD5A8383600D for ; Fri, 14 Jan 2022 14:59:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CD5A8383600D Received: by mail-pg1-x529.google.com with SMTP id v25so2978304pge.2 for ; Fri, 14 Jan 2022 06:59:19 -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=w8kSrBdnRnlzcfb4HN/J8r6G426dCo4HueA46fXA804=; b=RM8TNB9b3Jkp5zR1sjcrsPzXDDBsEFxPpCfA0zVVbO6zTCaT9V820zVrUUp2h2SsSq PuJgp5GSiaDzxnE6+cMmJR61Whm9z//2wAbW8VxKsnXlOrYlNJ54aZ/xuz9OWpGjtl3Z fkzc9ToaqJH9yx3nEN1836/81KbZijV7tODzSrJIovqQrNDYUZSbhhyc7Jxsvmcq2whc eFEXL8TcG+OHKZ9j2S58szqONiMe/yZ7jqvCGT6tx8YC4SNNLkJVVGa++TLtxSsVm5g0 FaEpIsLR61IBs5JHO9MiI7NpkG0IV5lehaJeSEK1KcgWDOg8kyalShlOmBktlirOQ+V2 07/A== X-Gm-Message-State: AOAM532HHV0siOJQG1wADrs680Rqtxe1a+crFy48pZuG9/mSCapL+nnf HWtgv1tFUgU0j575sVL1wmUsThCc1OxJMW/RNwz1brRkBAI= X-Google-Smtp-Source: ABdhPJxXSEzUYNXRvyOpYE8yXxfQd1iJlyPoG2VFxX8/ivvF40CoWRpGxg0No0BronX9TN/Cy1LQ/K7twa6ZsrlK/60= X-Received: by 2002:a05:6a00:2410:b0:4bc:dda9:2e92 with SMTP id z16-20020a056a00241000b004bcdda92e92mr9222351pfh.76.1642172358861; Fri, 14 Jan 2022 06:59:18 -0800 (PST) MIME-Version: 1.0 References: <20220111021241.1937265-1-hjl.tools@gmail.com> In-Reply-To: From: "H.J. Lu" Date: Fri, 14 Jan 2022 06:58:42 -0800 Message-ID: Subject: Re: [PATCH] elf: Remove the 1-page gap before the RELRO segment To: Alan Modra Cc: Binutils Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3022.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP 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: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Jan 2022 14:59:21 -0000 On Fri, Jan 14, 2022 at 12:12 AM Alan Modra wrote: > > On Thu, Jan 13, 2022 at 05:19:22AM -0800, H.J. Lu wrote: > > On Thu, Jan 13, 2022 at 4:52 AM Alan Modra wrote: > > > > > > On Mon, Jan 10, 2022 at 06:12:41PM -0800, H.J. Lu via Binutils wrote: > > > > The existing RELRO scheme may leave a 1-page gap before the RELRO segment > > > > and align the end of the RELRO segment to the page size: > > > > > > > > [18] .eh_frame PROGBITS 408fa0 008fa0 005e80 00 A 0 0 8 > > > > [19] .init_array INIT_ARRAY 410de0 00fde0 000008 08 WA 0 0 8 > > > > [20] .fini_array FINI_ARRAY 410de8 00fde8 000008 08 WA 0 0 8 > > > > [21] .dynamic DYNAMIC 410df0 00fdf0 000200 10 WA 7 0 8 > > > > [22] .got PROGBITS 410ff0 00fff0 000010 08 WA 0 0 8 > > > > [23] .got.plt PROGBITS 411000 010000 000048 08 WA 0 0 8 > > > > > > Do you know what is going wrong with the relro section layout for this > > > to occur? > > > > > > In this particular case, the end of the read-only segment is at > > > 0x408fa0 + 0x5e80 = 0x40ee20. My guess is that layout of the > > > following rw sections starts on the next page plus current offset > > > within page, the standard choice to minimise disk pages. ie. We start > > > at 0x40fe20. Then discover that this puts .got.plt at 0x40fe20 + 8 + > > > 8 + 0x200 + 0x10 = 0x40f040. However, we want this to be on a page > > > boundary so that the relro segment ends on a page boundary. So we > > > bump 0x40f040 up to 0x411000 and calculate backwards from there to > > > arrive at .init_array with a vma of 0x410de0. Resulting in the > > > 0x40f000 page being unused. > > > > > > If instead we start relro layout on the next page, we'd start laying > > > out at 0x40f000 rather than 0x40fe20. I think that would be the > > > > But if the prior ro section size is greater than one page, we want > > to subtract 1 page: > > > > + /* If the preceding section size is greater than the maximum > > + page size, subtract the maximum page size. Otherwise, > > + align the RELRO segment to the maximum page size. */ > > + if (prev_sec->size > seg->maxpagesize) > > + { > > + desired_end -= seg->maxpagesize; > > + relro_end -= seg->maxpagesize; > > + } > > + else > > + { > > + desired_end &= ~(seg->maxpagesize - 1); > > + relro_end &= ~(seg->maxpagesize - 1); > > + } > > + } > > The above code is the major reason why I took a dislike to your > patch. I fully expected you would have rev 2 posted. Why does > anything depend on the size of a previous section?? That just doesn't If the size of the preceding section is less than one page, we need to align the RELRO segment. If we subtract one page, we still have a 1-page gap. I ran out of time before I could dig into the complex logic. > make sense. And please don't write comments that just say what the > code is doing. Any half competent programmer can see what the code is > doing. Write comments that say *why*. > > > > correct thing to do rather than fixing up afterwards as your patch > > > does. > > > > > > > I am checking in my patch as Nick has approved it. There is a > > possibility that one 1-page gap may be needed to maintain > > the section alignment. My patch checks it and includes many > > testcase adjustments to remove one 1-page gap. Can you > > update the RELRO segment algorithm to make my fixup > > unnecessary? > > Yes, I do think that is possible and desirable. My thinking last > night was that it ought to be easy too. A one-liner even. Silly me, > a little experimentation soon showed up a fail of the pr18176 > testcase, demonstrating that there are cases we can save disk space > without causing gaps. > > -- > Alan Modra > Australia Development Lab, IBM -- H.J.