| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b35ca3a3-8de5-4d32-8d30-d4e562f6b0de@linux.ibm.com>
Date: Thu, 30 Jan 2025 16:47:00 +0100
From: Jens Remus <jremus@...ux.ibm.com>
To: Josh Poimboeuf <jpoimboe@...nel.org>, x86@...nel.org
Cc: Peter Zijlstra <peterz@...radead.org>,
Steven Rostedt <rostedt@...dmis.org>, Ingo Molnar <mingo@...nel.org>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
linux-kernel@...r.kernel.org, Indu Bhagat <indu.bhagat@...cle.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Namhyung Kim <namhyung@...nel.org>,
Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
linux-perf-users@...r.kernel.org, Mark Brown <broonie@...nel.org>,
linux-toolchains@...r.kernel.org, Jordan Rome <jordalgo@...a.com>,
Sam James <sam@...too.org>, linux-trace-kernel@...r.kernel.org,
Andrii Nakryiko <andrii.nakryiko@...il.com>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Florian Weimer <fweimer@...hat.com>, Andy Lutomirski <luto@...nel.org>,
Masami Hiramatsu <mhiramat@...nel.org>, Weinan Liu <wnliu@...gle.com>,
Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>
Subject: Re: [PATCH v4 19/39] unwind_user/sframe: Add support for reading
.sframe contents
On 22.01.2025 03:31, Josh Poimboeuf wrote:
> 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.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@...nel.org>
> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> +struct sframe_fre {
> + unsigned int size;
> + s32 ip_off;
The IP offset (from function start) in the SFrame V2 FDE is unsigned:
u32 ip_off;
> + s32 cfa_off;
> + s32 ra_off;
> + s32 fp_off;
> + u8 info;
> +};
...
> +#define __UNSAFE_GET_USER_INC(to, from, type, label) \
> +({ \
> + type __to; \
> + unsafe_get_user(__to, (type __user *)from, label); \
> + from += sizeof(__to); \
> + to = (typeof(to))__to; \
> +})
> +
> +#define UNSAFE_GET_USER_INC(to, from, size, label) \
> +({ \
> + switch (size) { \
> + case 1: \
> + __UNSAFE_GET_USER_INC(to, from, u8, label); \
> + break; \
> + case 2: \
> + __UNSAFE_GET_USER_INC(to, from, u16, label); \
> + break; \
> + case 4: \
> + __UNSAFE_GET_USER_INC(to, from, u32, label); \
> + break; \
> + default: \
> + return -EFAULT; \
> + } \
> +})
This does not work for the signed SFrame fields, such as the FRE CFA,
RA, and FP offsets, as it does not perform the required sign extension.
One option would be to rename to UNSAFE_GET_USER_UNSIGNED_INC() and
re-introduce UNSAFE_GET_USER_SIGNED_INC() using s8, s16, and s32.
> +static __always_inline int __read_fre(struct sframe_section *sec,
> + struct sframe_fde *fde,
> + unsigned long fre_addr,
> + struct sframe_fre *fre)
> +{
> + unsigned char fde_type = SFRAME_FUNC_FDE_TYPE(fde->info);
> + unsigned char fre_type = SFRAME_FUNC_FRE_TYPE(fde->info);
> + unsigned char offset_count, offset_size;
> + s32 ip_off, cfa_off, ra_off, fp_off;
The IP offset (from function start) in the SFrame V2 FRE is unsigned:
u32 ip_off;
s32 cfa_off, ra_off, fp_off;
> + unsigned long cur = fre_addr;
> + unsigned char addr_size;
> + 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);
Ok: The SFrame V2 FRE IP offset is unsigned u8, u16, or u32.
> + if (fde_type == SFRAME_FDE_TYPE_PCINC && ip_off > fde->func_size)
> + return -EFAULT;
> +
> + UNSAFE_GET_USER_INC(info, cur, 1, Efault);
Ok: The SFrame V2 FRE info word is one byte of data.
> + offset_count = SFRAME_FRE_OFFSET_COUNT(info);
> + offset_size = offset_size_enum_to_size(SFRAME_FRE_OFFSET_SIZE(info));
> + if (!offset_count || !offset_size)
> + return -EFAULT;
> +
> + if (cur + (offset_count * offset_size) > sec->fres_end)
> + return -EFAULT;
> +
> + fre->size = addr_size + 1 + (offset_count * offset_size);
> +
> + UNSAFE_GET_USER_INC(cfa_off, cur, offset_size, Efault);
Issue: The SFrame V2 FRE CFA offset is signed s8, s16, or s32. Sign
extension required when storing in s32.
> + offset_count--;
> +
> + ra_off = sec->ra_off;
> + if (!ra_off) {
> + if (!offset_count--)
> + return -EFAULT;
> +
> + UNSAFE_GET_USER_INC(ra_off, cur, offset_size, Efault);
Issue: The SFrame V2 FRE RA offset is signed s8, s16, or s32. Sign
extension required when storing in s32.
> + }
> +
> + fp_off = sec->fp_off;
> + if (!fp_off && offset_count) {
> + offset_count--;
> + UNSAFE_GET_USER_INC(fp_off, cur, offset_size, Efault);
Issue: The SFrame V2 FRE FP offset is signed s8, s16, or s32. Sign
extension required when storing in s32.
> + }
> +
> + if (offset_count)
> + return -EFAULT;
> +
> + 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;
> +}
> +
> +static __always_inline int __find_fre(struct sframe_section *sec,
> + struct sframe_fde *fde, unsigned long ip,
> + struct unwind_user_frame *frame)
> +{
> + unsigned char fde_type = SFRAME_FUNC_FDE_TYPE(fde->info);
> + struct sframe_fre *fre, *prev_fre = NULL;
> + struct sframe_fre fres[2];
> + unsigned long fre_addr;
> + bool which = false;
> + unsigned int i;
> + s32 ip_off;
> +
> + ip_off = (s32)(ip - sec->sframe_start) - fde->start_addr;
The IP offset (from function start) in the SFrame V2 FRE is unsigned.
The function start address offset (from .sframe section begin) is signed.
Therefore:
u32 ip_off;
ip_off = ip - (sec->sframe_start + fde->start_addr);
> +
> + if (fde_type == SFRAME_FDE_TYPE_PCMASK)
> + ip_off %= fde->rep_size;
Following is a patch with the suggested changes, that were required to
make unwinding of user space using SFrame work on s390:
diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index bba14c5fe0f5..ea2d491ea68f 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -19,7 +19,7 @@
struct sframe_fre {
unsigned int size;
- s32 ip_off;
+ u32 ip_off;
s32 cfa_off;
s32 ra_off;
s32 fp_off;
@@ -136,7 +136,26 @@ static __always_inline int __find_fde(struct sframe_section *sec,
to = (typeof(to))__to; \
})
-#define UNSAFE_GET_USER_INC(to, from, size, label) \
+#define UNSAFE_GET_USER_SIGNED_INC(to, from, size, label) \
+({ \
+ switch (size) { \
+ case 1: \
+ __UNSAFE_GET_USER_INC(to, from, s8, label); \
+ break; \
+ case 2: \
+ __UNSAFE_GET_USER_INC(to, from, s16, label); \
+ break; \
+ case 4: \
+ __UNSAFE_GET_USER_INC(to, from, s32, label); \
+ break; \
+ default: \
+ dbg_sec_uaccess("%d: bad UNSAFE_GET_USER_SIGNED__INC size %u\n",\
+ __LINE__, size); \
+ return -EFAULT; \
+ } \
+})
+
+#define UNSAFE_GET_USER_UNSIGNED_INC(to, from, size, label) \
({ \
switch (size) { \
case 1: \
@@ -149,7 +168,7 @@ static __always_inline int __find_fde(struct sframe_section *sec,
__UNSAFE_GET_USER_INC(to, from, u32, label); \
break; \
default: \
- dbg_sec_uaccess("%d: bad UNSAFE_GET_USER_INC size %u\n",\
+ dbg_sec_uaccess("%d: bad UNSAFE_GET_USER_UNSIGNED_INC size %u\n",\
__LINE__, size); \
return -EFAULT; \
} \
@@ -163,7 +182,8 @@ static __always_inline int __read_fre(struct sframe_section *sec,
unsigned char fde_type = SFRAME_FUNC_FDE_TYPE(fde->info);
unsigned char fre_type = SFRAME_FUNC_FRE_TYPE(fde->info);
unsigned char offset_count, offset_size;
- s32 ip_off, cfa_off, ra_off, fp_off;
+ u32 ip_off;
+ s32 cfa_off, ra_off, fp_off;
unsigned long cur = fre_addr;
unsigned char addr_size;
u8 info;
@@ -179,14 +199,14 @@ static __always_inline int __read_fre(struct sframe_section *sec,
return -EFAULT;
}
- UNSAFE_GET_USER_INC(ip_off, cur, addr_size, Efault);
+ UNSAFE_GET_USER_UNSIGNED_INC(ip_off, cur, addr_size, Efault);
if (fde_type == SFRAME_FDE_TYPE_PCINC && ip_off > fde->func_size) {
dbg_sec_uaccess("fre starts past end of function: ip_off=0x%x, func_size=0x%x\n",
ip_off, fde->func_size);
return -EFAULT;
}
- UNSAFE_GET_USER_INC(info, cur, 1, Efault);
+ UNSAFE_GET_USER_UNSIGNED_INC(info, cur, 1, Efault);
offset_count = SFRAME_FRE_OFFSET_COUNT(info);
offset_size = offset_size_enum_to_size(SFRAME_FRE_OFFSET_SIZE(info));
if (!offset_count || !offset_size) {
@@ -200,7 +220,7 @@ static __always_inline int __read_fre(struct sframe_section *sec,
fre->size = addr_size + 1 + (offset_count * offset_size);
- UNSAFE_GET_USER_INC(cfa_off, cur, offset_size, Efault);
+ UNSAFE_GET_USER_SIGNED_INC(cfa_off, cur, offset_size, Efault);
offset_count--;
ra_off = sec->ra_off;
@@ -210,13 +230,13 @@ static __always_inline int __read_fre(struct sframe_section *sec,
return -EFAULT;
}
- UNSAFE_GET_USER_INC(ra_off, cur, offset_size, Efault);
+ UNSAFE_GET_USER_SIGNED_INC(ra_off, cur, offset_size, Efault);
}
fp_off = sec->fp_off;
if (!fp_off && offset_count) {
offset_count--;
- UNSAFE_GET_USER_INC(fp_off, cur, offset_size, Efault);
+ UNSAFE_GET_USER_SIGNED_INC(fp_off, cur, offset_size, Efault);
}
if (offset_count) {
@@ -247,9 +267,9 @@ static __always_inline int __find_fre(struct sframe_section *sec,
unsigned long fre_addr;
bool which = false;
unsigned int i;
- s32 ip_off;
+ u32 ip_off;
- ip_off = (s32)(ip - sec->sframe_start) - fde->start_addr;
+ ip_off = ip - (sec->sframe_start + fde->start_addr);
if (fde_type == SFRAME_FDE_TYPE_PCMASK)
ip_off %= fde->rep_size;
Regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
+49-7031-16-1128 Office
jremus@...ibm.com
IBM
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
Powered by blists - more mailing lists