Received-SPF: pass (google.com: domain of linux-kernel+bounces-27015-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <578aae7c-4069-4071-ba4b-cc86d3b516c1@siemens.com>
Date: Tue, 16 Jan 2024 06:21:37 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] riscv/efistub: Ensure GP-relative addressing is not used
Content-Language: en-US
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Palmer Dabbelt <palmer@rivosinc.com>, linux-efi@vger.kernel.org,
 linux-kernel@vger.kernel.org
References: <mhng-8a4a5f85-faf9-405c-95a0-78cd04ec6509@palmer-ri-x1c9>
 <56b52d4e-9dc0-400c-a141-7e70f5c72afa@siemens.com>
 <CAMj1kXGR1aQdvej+0drfim-ZP27ZhO9UR_i_PT6F+hsV0UvbJw@mail.gmail.com>
From: Jan Kiszka <jan.kiszka@siemens.com>
In-Reply-To: <CAMj1kXGR1aQdvej+0drfim-ZP27ZhO9UR_i_PT6F+hsV0UvbJw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?Wk5jVmRXZ3crZkQrY2h1TEpVamxHcTdFTUYwbUEvUzV3SWhKcXBPMis4Y2dq?=
 =?utf-8?B?cENxVmY0SllTMEY2bjc3RElNdWxaWUNsWER1Y254Zk1YV3hLU1dHczFNalVs?=
 =?utf-8?B?TnZQVUk0MzVTNnVBQUtoUncyNG12TmRlZEZPMldvSHJNeWJkR1QxM0hldWpt?=
 =?utf-8?B?UDVDZmw1UkRCWGh3SCtZOHFiQW9jamN4V0w0SnRwNTUxZm5yVWZ1bnNGZ05w?=
 =?utf-8?B?Ylh4MGl5dmtaVmRQbjNRcDQzamxEMk5TWjRqMnVxc2tJSERyYkQrcHJCbXM4?=
 =?utf-8?B?ZDJKR1hxNUFEYW5GbmRCOVBpVUJ5YjE1ZjIrUGdBY2FHOENiWDAvajlHdGEv?=
 =?utf-8?B?UlVwMXkyTmx1OGRrRDdXMzV0ZmZ4YStqZjl2am1oTHJ0R3I2S09ZS3hFa2Qw?=
 =?utf-8?B?MTJKZXIrNThCMXNmYUM3Yk1ZbUNOdVIzOGlSNG9FRE8wRmRYYmNKV3kyTlpo?=
 =?utf-8?B?K1hpbG5TYXJqVnJGVDdaZGljWmlSOVA1RFFIZ0FXd3BXTWdjUjh3dkwxL2Rs?=
 =?utf-8?B?ckZrc055RDF2NitXQndGSW5QRUhseGRJQ3FpY0lwamZuODNSWGlpQk5WN0Vj?=
 =?utf-8?B?SitTRWVoa2p2eGlHMFRER2doK0htMVJKQkVTVHhtVFFTQ0VESjNxVDZOeXJG?=
 =?utf-8?B?S3hsZmpQZmpYR0NFa3Z4SzFzcThCZGNaeW9oTnZzdVZ6eUlrdGZ0S3BOS3R0?=
 =?utf-8?B?SmpQSTNMV3o3L3hMYzJxampmdVhQU0xCaDBZckhaRkd5d20zSElRL2lzYlNa?=
 =?utf-8?B?a1FKeWpSSEZpL3FLNFhQbVJ5aHBvRk85aHoxQUdiUUh0dndNSHEvY09xMCth?=
 =?utf-8?B?NCt2RWVJRnFHM3hqU2lYcXkzREtwd2E4d3NIeFllWFJWS1ZGUFZDSkdpRWN6?=
 =?utf-8?B?NHhCcFpnTjU2OWlKWURLR05LWjZwcTVUckR5NC9XWm5xS0tDSm14VEo5c201?=
 =?utf-8?B?K05UdlV1M3BDeEtEbURpTG05N2hpWHVBaFcwblJpL0dqUUwxODlZa25QMSs0?=
 =?utf-8?B?Ry9CYVJKNXpnVWRYbjlzdFE1RU5OQUQzd3NKRmxyakxLamFIYWxNeldDMk16?=
 =?utf-8?B?eGthYm9VWnBTQ2Z5a09DSEV4T3U3S0QrK1Q2T3pMbm1JYmFEaFVpcnVaQzgw?=
 =?utf-8?B?ekxrRHFyMkg4dTVKdXZFMi92aUViN1VxSHBDNGJ2NzFEK1ZHSk80UEFHT1g5?=
 =?utf-8?B?Q214KzdaYkI1NVp3elViTGZDTWZvbDVUdkN1c2J6Y3Y2bTVFcUcySmR2QTNR?=
 =?utf-8?B?Z2NGR0Y0QUpNa0Ywb0l1UENhYUtqZmRubEpOR1lWZkoveGo4M3A2dnRTNVNP?=
 =?utf-8?B?SDFpUFlMbU0yRGtBdHgrRG43MTc2K0JudEk3ak1RTkI4dkRZUHFKa2NOSW9p?=
 =?utf-8?B?QW9xRUV4bk9zTVk5V2c4UDRoQjZEZlVvU253Q0VqZ0VjSnBiY3RxdEcwNloz?=
 =?utf-8?B?bXI1Q0wvOVBiRm1wbXdCanRKK0ZEb1YrQm9KRmxlZTdINnErN0lOeVFFb3RD?=
 =?utf-8?B?akJOTVlkYXcvM1BqODJaR0JaZ3FjbmsxR1BHNUhNbXdZUzVxUVppY0lYUVBB?=
 =?utf-8?B?ajlOOGRBYnR2b29Lb2c1UlpaUGtRdEYyUXEyaklwbDZOelZuTFhEWDZjZmxE?=
 =?utf-8?B?TnVWS2t1dFc1SEE1aTA3S0RMNGpKSXZnbXNadWtlN0lGYWZodWdXYitmOXVS?=
 =?utf-8?B?bm5hNTdtMEhxaWppaDhhVC9paFVkY2NUczllUStzc2lZTVQ4R216bWNJcXMw?=
 =?utf-8?B?M1lnRlRBby94MlRaUzBMeGg4SklmbGI0OVk0QTlIZ3NwejMwUExIMWZpNVBP?=
 =?utf-8?B?a3M1SzJCRmR4bkZoWVh0K2dET0pQT3dsV3RKYVZSOGJoUFBhbXhiVzBRazZv?=
 =?utf-8?B?RzVNdEhEcytPa2xUM05IU3pvUzlJSmNiRjVKbENzbHZGeXBsTGgzTndTMEJs?=
 =?utf-8?B?TlpmVnRtWmJXem5XUm9OVUw1amRldEZqcGRCRWZyMlluQk5FWWNPam1yckdI?=
 =?utf-8?B?cTh3QlVINW1aeWtBQko5QjRQRCsvY2lIbnVBUExDZS82UG1BRkdFUjFmb3Fz?=
 =?utf-8?B?Q21odHpZbWJXM1g4TnExSnFVTXdYZmtsRWVZaUJ6RERVdHRPTEEyWFZIMXp1?=
 =?utf-8?B?ZE1hTnBFOWxYTEwxU293UWtORVV1VUlrWFFINmNOTVVyZlRGSXI5RGluSzd2?=
 =?utf-8?B?a1E9PQ==?=
X-OriginatorOrg: siemens.com
X-MS-Exchange-CrossTenant-Network-Message-Id: fbad8732-8d3f-4acb-3db7-08dc1653047d
X-MS-Exchange-CrossTenant-AuthSource: AS4PR10MB6181.EURPRD10.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Jan 2024 05:21:39.5044
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 38ae3bcd-9579-4fd4-adda-b42e1495d55a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: IImCW02sKuqIvIGcfthuDZTUfmTWeXJSVIN+9u5pII5+WDUfpE1hQdkwye4+Q1UdlJ6BM62U4hXeX84rqP9QMQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS1PR10MB5601

On 15.01.24 18:34, Ard Biesheuvel wrote:
> On Sat, 13 Jan 2024 at 11:35, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>
>> On 12.01.24 19:56, Palmer Dabbelt wrote:
>>> On Fri, 12 Jan 2024 10:51:16 PST (-0800), Ard Biesheuvel wrote:
>>>> Hi Jan,
>>>>
>>>> On Fri, 12 Jan 2024 at 19:37, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>
>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>
>>>>> The cflags for the RISC-V efistub were missing -mno-relax, thus were
>>>>> under the risk that the compiler could use GP-relative addressing. That
>>>>> happened for _edata with binutils-2.41 and kernel 6.1, causing the
>>>>> relocation to fail due to an invalid kernel_size in handle_kernel_image.
>>>>> It was not yet observed with newer versions, but that may just be luck.
>>>>>
>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>> ---
>>>>>
>>>>> Something like this should go to stable as well, but we will need
>>>>> rebased patches.
>>>>>
>>>>>  drivers/firmware/efi/libstub/Makefile | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/firmware/efi/libstub/Makefile
>>>>> b/drivers/firmware/efi/libstub/Makefile
>>>>> index 06964a3c130f..d561d7de46a9 100644
>>>>> --- a/drivers/firmware/efi/libstub/Makefile
>>>>> +++ b/drivers/firmware/efi/libstub/Makefile
>>>>> @@ -28,7 +28,7 @@ cflags-$(CONFIG_ARM)          += -DEFI_HAVE_STRLEN
>>>>> -DEFI_HAVE_STRNLEN \
>>>>>                                    -DEFI_HAVE_MEMCHR
>>>>> -DEFI_HAVE_STRRCHR \
>>>>>                                    -DEFI_HAVE_STRCMP -fno-builtin
>>>>> -fpic \
>>>>>                                    $(call
>>>>> cc-option,-mno-single-pic-base)
>>>>> -cflags-$(CONFIG_RISCV)         += -fpic -DNO_ALTERNATIVE
>>>>> +cflags-$(CONFIG_RISCV)         += -fpic -DNO_ALTERNATIVE -mno-relax
>>>>
>>>> Can we detect the presence of these references (via the relocation
>>>> type)? We already do something similar for ordinary absolute
>>>> references too.
>>>
>>> If there's no `__global_pointer$` symbol then the linker won't make
>>> GP-relative relaxations (because it doesn't know where GP is).  We
>>> usually define that symbol in the linker script, but I'm not entierly
>>> sure how libstub gets its linker script...
>>>
>>
>> The stub seems to be linked together with the rest of the kernel, thus
>> the regular arch/riscv/kernel/vmlinux.lds.S is used.
>>
> 
> Indeed - the EFI stub is part of the same executable as vmlinux, we
> just mangle the symbol names to ensure that only code that can be
> safely called from the EFI stub can be linked to it.
> 
> If the effect of -mno-relax is to stop emitting R_RISCV_RELAX
> relocations, we should perhaps add those to the STUBCOPY_RELOC-y
> Makefile variable? (in the same file). BTW R_RISCV_HI20 doesn't seem
> like the right value there to begin with: the idea of that is to
> disallow ELF relocations that evaluate to expressions that can only be
> known at runtime (like absolute addresses for global pointer
> variables)

How to do that best? Simply replace R_RISCV_HI20 with R_RISCV_RELAX?

Jan

-- 
Siemens AG, Technology
Linux Expert Center