From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by mx.groups.io with SMTP id smtpd.web10.28983.1683297226042106852 for ; Fri, 05 May 2023 07:33:46 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="signature has expired" header.i=@bsdio.com header.s=fm1 header.b=jJFhb5/N; spf=pass (domain: bsdio.com, ip: 66.111.4.25, mailfrom: rebecca@bsdio.com) Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.nyi.internal (Postfix) with ESMTP id 5266F5C0090; Fri, 5 May 2023 10:33:45 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute4.internal (MEProxy); Fri, 05 May 2023 10:33:45 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdio.com; h=cc :cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to; s=fm1; t= 1683297225; x=1683383625; bh=NF2uU9usPDpym5/vZFYOx3xLQ7MPWjOvy5D 2Uy5VRJg=; b=jJFhb5/NomO0bF3wQ5hmO3EUAti6tjxKvFOx2HXhb/oElhveiCh m9s7tsDoG6yK6OEoJ0BZv2cWctaNmj++3eaYzsGsa2ZPdiM8/WgO0tmaR28C7QVN 8pVKn/jBNU0b3BGvktgPs43D+XcFFd+7pOuGzoiLNv2mpwEMXRNWkk1z50pP1Eif 9dNxOTiqkZdaYttLM59PtHd5XxcNt/jJ+l/L4Gh3t4k4sj8PLm4wP+mNiLeCQ0MZ m9Kiys2SLbEz8ysETQvFSJMeznEuaUFmpLyrsgg1rlP6JVWfk3e0iuHk0c2zH0Ji /MecJKYkT4nsxqcdz0nNDsVX/SW8GF5Wy9g== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t= 1683297225; x=1683383625; bh=NF2uU9usPDpym5/vZFYOx3xLQ7MPWjOvy5D 2Uy5VRJg=; b=Ar/82nY4ovEjiIIA0HPPoFubmNtwfc195jNM5vjm6brg5BAokcU mVMA9/Xf6Zw9pH+SX9pT3ys4TzidNzjVVbePeOCctZ+ApkDwQf4ynVyk4o6nkVxX 4OR/5l8eIZpToH1vP/kkJxgsyI84QCGoLVrSPyOOiPp8hHdOkX31wB8+8eclndag K1V6WDN/ob/MXQCf7czQA5xryMvf6UuWhm2VDY2WLHXWLb9mAR/+jK48ZJfssMmW Q4XW7rFrTX1EEVKWa2WqAarowBLM6VizbMIyZQyRl/87GEMD+/GPl6WWw6FNqiQJ PNRC95eJpe/2bilNOYY26cGUkGVzRbO5ghg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrfeefvddgjeekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepkfffgggfuffvvehfhfgjtgfgsehtkeertddtfeejnecuhfhrohhmpeftvggs vggttggrucevrhgrnhcuoehrvggsvggttggrsegsshguihhordgtohhmqeenucggtffrrg htthgvrhhnpeelfffffefgteelleelhfffueejledvjeevieeuieetvdehleevvdfgveel vdekhfenucffohhmrghinhepghhithhhuhgsrdgtohhmnecuvehluhhsthgvrhfuihiivg eptdenucfrrghrrghmpehmrghilhhfrhhomheprhgvsggvtggtrgessghsughiohdrtgho mh X-ME-Proxy: Feedback-ID: i5b994698:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 5 May 2023 10:33:42 -0400 (EDT) Message-ID: <5faf3ba0-0d75-f526-6491-30c2c96ad186@bsdio.com> Date: Fri, 5 May 2023 08:33:41 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: [PATCH v5 02/10] MdePkg: don't set visibility to hidden To: =?UTF-8?Q?Marvin_H=c3=a4user?= , Ard Biesheuvel , Gerd Hoffmann Cc: devel@edk2.groups.io, Yuwei Chen , Oliver Steffen , Bob Feng , Daniel Schaefer , Chao Li , Dongyan Qian , Michael D Kinney , Pawel Polawski , Baoqi Zhang , Leif Lindholm , Zhiguang Liu , Sunil V L , Ard Biesheuvel , Liming Gao References: From: "Rebecca Cran" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Gerd, Does this series need rework following this discussion, or is it ready to merge? -- Rebecca Cran On 4/21/23 01:46, Marvin Häuser wrote: > >> On 21. Apr 2023, at 09:21, Ard Biesheuvel wrote: >> >> On Fri, 21 Apr 2023 at 08:49, Gerd Hoffmann wrote: >>> >>> On Fri, Apr 21, 2023 at 06:01:11AM +0000, Marvin Häuser wrote: >>>> >>>>> On 21. Apr 2023, at 06:45, Gerd Hoffmann wrote: >>>>> >>>>> Not needed any more on modern toolchains, they are better >>>>> in not creating a GOT without this trick. >>>> >>>> Hi Gerd, >>>> >>>> Thanks! Just out of interest, how did you test this and what were >>>> the results? >>> >>> Patch #1, adding a linker script assert as suggested by ard, then: >>> >>> * compile + test on my local workstation (fedora 37, gcc 12). >>> * run CI >>> * compile on some older distros: >>>   - rhel-8 (gcc 8) >>>   - ubuntu-18.04 (gcc 7) >>> >> >> I just realized that on x86, GenFw has some code to deal with GOT >> entries if they are emitted. I'm not sure how often that gets >> exercised, given our prior use of hidden visibility, but at least the >> GOT entries should be covered by relocations if they exist. >> >> *However*, one thing we are not taking into account is the fact that >> relaxations are not usually reflected in the relocations emitted by >> the compiler when using --emit-relocs. So we might end up with >> occurrences like the below (taken from the Linux kernel but the idea >> is the same) >> >> ffffffff82fa59d5:       4c 8d 0d 24 66 88 ff    lea >>    -0x7799dc(%rip),%r9 >>      ffffffff82fa59d8: R_X86_64_REX_GOTPCRELX >>        level4_kernel_pgt-0x4 >> ffffffff82fa59dc:       49 8d 69 67             lea    0x67(%r9),%rbp >> ffffffff82fa59e0:       4c 8d 15 19 76 88 ff    lea >>    -0x7789e7(%rip),%r10 >>      ffffffff82fa59e3: R_X86_64_REX_GOTPCRELX >>        level3_kernel_pgt-0x4 >> >> So here, the GOT loads have been relaxed into LEA instructions, but >> GenFw will decode the immediate and assume it points to the GOT entry >> rather than the variable itself, and happily emit a PE relocation for >> it. >> >> So it would be better to ASSERT() on non-empty GOT, and ignore such >> GOTPCREL relocations instead of attempting to relocate the GOT entries >> they (used to) refer to. > > Hmm, we’ve been toying with using only PIE relocs for X64 for a bit > and finally merged it into master, so far no issues: > https://github.com/acidanthera/audk/commit/92bb32130bcd0c35e48bdc308a18e5bc74cbaa42 > https://github.com/acidanthera/audk/commit/42988773a06f9d6bf345fcbe82c1082ff1cfa2af > > In fact (I *did not* confirm this, it’s only a report I got), it seems > to fix something regarding the stack protector. I’d not be surprised > if there are edge-cases where -q does not get all necessary relocs > when PIE is enabled. > > Best regards, > Marvin