Re: [PATCH v3 7/8] sframe: Introduce in-kernel SFRAME_VALIDATION.

On 4/21/2026 3:29 AM, Dylan Hatch wrote:
> On Mon, Apr 20, 2026 at 5:31 AM Jens Remus <[email protected]> wrote:
>>
>> On 4/20/2026 7:02 AM, Dylan Hatch wrote:
>>> On Thu, Apr 16, 2026 at 8:04 AM Jens Remus <[email protected]> wrote:
>>>> On 4/6/2026 8:49 PM, Dylan Hatch wrote:
>>
>>>>> Generalize the __safe* helpers to support a non-user-access code path.
>>>>> Allow for kernel FDE read failures due to the presence of .rodata.text.
>>>>> This section contains code that can't be executed by the kernel
>>>>> direclty, and thus lies ouside the normal kernel-text bounds.
>>>>
>>>> Nits: s/direclty/directly/ s/ouside/outside/
>>>>
>>>> Could you please explain the issue?  How/why does .sframe for
>>>> .rodata.text pose an issue for .sframe verification?
>>>
>>> __read_fde checks that the fde_addr it extracts is within the bounds
>>> of sec->text_start and sec->text_end. In the case of the vmlinux
>>
>> Looking at the existing check in __read_fde(), do you agree that it is
>> wrong, as sec->text_end IIUC points behind .text and thus the check
>> should be:
>>
>>         if (func_addr < sec->text_start || func_addr >= sec->text_end)
>>                 return -EINVAL;
> 
> I agree this is correct. Is this a fix that would be folded into your
> previous patch series?

Yes.  I would send a new version once we have clarified how to move
forward in general.

>>> Alternatively, we can check for FDEs located
>>> in .rodata.text during validation, but this seems to only be present
>>> in arm64, so maybe we would need an arch-specific hook to do this? I'm
>>> open to suggestions.
>>
>> Maybe that is better than ignoring __read_fde() failures?  I first
>> thought this would get nasty, but maybe it would not be too bad.
>> Following is what I came up with (note tabs replaced by spaces due to
>> copy&paste from terminal):

>> diff --git a/include/linux/sframe.h b/include/linux/sframe.h
>> @@ -63,6 +63,10 @@ struct sframe_section {
>>         unsigned long           sframe_end;
>>         unsigned long           text_start;
>>         unsigned long           text_end;
>> +#if defined(CONFIG_SFRAME_UNWINDER) && defined(CONFIG_ARM64)
>> +       unsigned long           rodatatext_start;
>> +       unsigned long           rodatatext_end;
>> +#endif
> 
> It looks to me like .rodata.text only exists for vmlinux. I wonder if
> in sframe_func_start_addr_valid we can just use the global
> _srodatatext and _erodatatext after identifying if an sframe_section
> corresponds to vmlinux (kernel_sfsec)? That way we don't need to add
> these extra fields.

Sure.  I don't have and preferences.

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
[email protected] / [email protected]

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/