On 5/5/2026 2:17 PM, Jens Remus wrote:
> From: Josh Poimboeuf <[email protected]>
>
> In preparation for using sframe to unwind user space stacks, add an
> sframe_find() interface for finding the sframe information associated
> with a given text address.
>
> For performance, use user_read_access_begin() and the corresponding
> unsafe_*() accessors. Note that use of pr_debug() in uaccess-enabled
> regions would break noinstr validation, so there aren't any debug
> messages yet. That will be added in a subsequent commit.
>
> Link:
> https://lore.kernel.org/all/77c0d1ec143bf2a53d66c4ecb190e7e0a576fbfd.1737511963.git.jpoim...@kernel.org/
> Link:
> https://lore.kernel.org/all/[email protected]/
>
> [ Jens Remus: Add initial support for SFrame V3 (limited to regular
> FDEs). Add support for PC-relative FDE function start offset. Simplify
> logic by using an internal FDE representation. Rename struct sframe_fre
> to sframe_fre_internal to align with struct sframe_fde_internal.
> Cleanup includes. Fix checkpatch errors "spaces required around that
> ':'". ]
> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
It just occurred to me that that the use of the return values -EINVAL
and -EFAULT is inconsistent across __read_fde() and __read_fre():
-EINVAL is to be used when a lookup for an IP fails (i.e. cannot be
satisfied) and
-EFAULT is to be used when the .sframe is corrupted/inconsistent and
should be unregistered.
> +static __always_inline int __read_fde(struct sframe_section *sec,
> + unsigned int fde_num,
> + struct sframe_fde_internal *fde)
> +{
> + unsigned long fde_addr, fda_addr, func_addr;
> + struct sframe_fde_v3 _fde;
> + struct sframe_fda_v3 _fda;
> +
> + fde_addr = sec->fdes_start + (fde_num * sizeof(struct sframe_fde_v3));
> + unsafe_copy_from_user(&_fde, (void __user *)fde_addr,
> + sizeof(struct sframe_fde_v3), Efault);
> +
> + func_addr = fde_addr + _fde.func_start_off;
> + if (func_addr < sec->text_start || func_addr >= sec->text_end)
> + return -EINVAL;
return -EFAULT;
> +
> + fda_addr = sec->fres_start + _fde.fres_off;
> + if (fda_addr + sizeof(struct sframe_fda_v3) > sec->fres_end)
> + return -EINVAL;
return -EFAULT;
> + unsafe_copy_from_user(&_fda, (void __user *)fda_addr,
> + sizeof(struct sframe_fda_v3), Efault);
> +
> + fde->func_addr = func_addr;
> + fde->func_size = _fde.func_size;
> + fde->fda_off = _fde.fres_off;
> + fde->fres_off = _fde.fres_off + sizeof(struct sframe_fda_v3);
> + fde->fres_num = _fda.fres_num;
> + fde->info = _fda.info;
> + fde->info2 = _fda.info2;
> + fde->rep_size = _fda.rep_size;
> +
> + return 0;
> +
> +Efault:
> + return -EFAULT;
> +}
> +static __always_inline int __read_fre(struct sframe_section *sec,
> + struct sframe_fde_internal *fde,
> + unsigned long fre_addr,
> + struct sframe_fre_internal *fre)
> +{
> + unsigned char fde_type = SFRAME_V3_FDE_TYPE(fde->info2);
> + unsigned char fde_pctype = SFRAME_V3_FDE_PCTYPE(fde->info);
> + unsigned char fre_type = SFRAME_V3_FDE_FRE_TYPE(fde->info);
> + unsigned char dataword_count, dataword_size;
> + s32 cfa_off, ra_off, fp_off;
> + unsigned long cur = fre_addr;
> + unsigned char addr_size;
> + u32 ip_off;
> + u8 info;
> +
> + addr_size = fre_type_to_size(fre_type);
> + if (!addr_size)
> + return -EFAULT;
> +
> + if (fre_addr + addr_size + 1 > sec->fres_end)
> + return -EFAULT;
> +
> + UNSAFE_GET_USER_INC(ip_off, cur, addr_size, Efault);
> + if (fde_pctype == SFRAME_FDE_PCTYPE_INC && ip_off > fde->func_size)
> + return -EFAULT;
> +
> + UNSAFE_GET_USER_INC(info, cur, 1, Efault);
> + dataword_count = SFRAME_V3_FRE_DATAWORD_COUNT(info);
> + dataword_size =
> dataword_size_enum_to_size(SFRAME_V3_FRE_DATAWORD_SIZE(info));
> + if (!dataword_count || !dataword_size)
> + return -EFAULT;
> +
> + if (cur + (dataword_count * dataword_size) > sec->fres_end)
> + return -EFAULT;
> +
> + /* TODO: Support for flexible FDEs not implemented yet. */
> + if (fde_type != SFRAME_FDE_TYPE_DEFAULT)
> + return -EFAULT;
> +
> + UNSAFE_GET_USER_INC(cfa_off, cur, dataword_size, Efault);
> + dataword_count--;
> +
> + ra_off = sec->ra_off;
> + if (!ra_off) {
> + if (!dataword_count--)
> + return -EFAULT;
> +
> + UNSAFE_GET_USER_INC(ra_off, cur, dataword_size, Efault);
> + }
> +
> + fp_off = sec->fp_off;
> + if (!fp_off && dataword_count) {
> + dataword_count--;
> + UNSAFE_GET_USER_INC(fp_off, cur, dataword_size, Efault);
> + }
> +
> + if (dataword_count)
> + return -EFAULT;
> +
> + fre->size = addr_size + 1 + (dataword_count * dataword_size);
> + fre->ip_off = ip_off;
> + fre->cfa_off = cfa_off;
> + fre->ra_off = ra_off;
> + fre->fp_off = fp_off;
> + fre->info = info;
> +
> + return 0;
> +
> +Efault:
> + return -EFAULT;
> +}
Similar for sframe_init_cfa_rule_data() (and sframe_init_rule_data())
that get introduced with subsequent commits.
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/
