[PATCH v4 26/39] unwind_user/sframe: Enable debugging in uaccess regions

Josh Poimboeuf
2025-01-22T02:32:19Z

Objtool warns about calling pr_debug() from uaccess-enabled regions, and
rightfully so. Add a dbg_sec_uaccess() macro which temporarily disables
uaccess before doing the dynamic printk, and use that to add debug
messages throughout the uaccess-enabled regions.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
kernel/unwind/sframe.c | 59 ++++++++++++++++++++++++++++--------
kernel/unwind/sframe_debug.h | 31 +++++++++++++++++++
2 files changed, 77 insertions(+), 13 deletions(-)

diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index f463123f9afe..a2ca26b952d3 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -53,12 +53,15 @@ static __always_inline int __read_fde(struct sframe_section *sec,
sizeof(struct sframe_fde), Efault);

ip = sec->sframe_start + fde->start_addr;
- if (ip < sec->text_start || ip > sec->text_end)
+ if (ip < sec->text_start || ip > sec->text_end) {
+ dbg_sec_uaccess("bad fde num %d\n", fde_num);
return -EINVAL;
+ }

return 0;

Efault:
+ dbg_sec_uaccess("fde %d usercopy failed\n", fde_num);
return -EFAULT;
}

@@ -85,16 +88,22 @@ static __always_inline int __find_fde(struct sframe_section *sec,
unsafe_get_user(func_off, (s32 __user *)mid, Efault);

if (ip_off >= func_off) {
- if (func_off < func_off_low)
+ if (func_off < func_off_low) {
+ dbg_sec_uaccess("fde %u not sorted\n",
+ (unsigned int)(mid - first));
return -EFAULT;
+ }

func_off_low = func_off;

found = mid;
low = mid + 1;
} else {
- if (func_off > func_off_high)
+ if (func_off > func_off_high) {
+ dbg_sec_uaccess("fde %u not sorted\n",
+ (unsigned int)(mid - first));
return -EFAULT;
+ }

func_off_high = func_off;

@@ -140,6 +149,8 @@ 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",\
+ __LINE__, size); \
return -EFAULT; \
} \
})
@@ -158,24 +169,34 @@ static __always_inline int __read_fre(struct sframe_section *sec,
u8 info;

addr_size = fre_type_to_size(fre_type);
- if (!addr_size)
+ if (!addr_size) {
+ dbg_sec_uaccess("bad addr_size in fde info %u\n", fde->info);
return -EFAULT;
+ }

- if (fre_addr + addr_size + 1 > sec->fres_end)
+ if (fre_addr + addr_size + 1 > sec->fres_end) {
+ dbg_sec_uaccess("fre addr+info goes past end of subsection\n");
return -EFAULT;
+ }

UNSAFE_GET_USER_INC(ip_off, cur, addr_size, Efault);
- if (fde_type == SFRAME_FDE_TYPE_PCINC && ip_off > fde->func_size)
+ 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);
offset_count = SFRAME_FRE_OFFSET_COUNT(info);
offset_size = offset_size_enum_to_size(SFRAME_FRE_OFFSET_SIZE(info));
- if (!offset_count || !offset_size)
+ if (!offset_count || !offset_size) {
+ dbg_sec_uaccess("zero offset_count or size in fre info %u\n",info);
return -EFAULT;
-
- if (cur + (offset_count * offset_size) > sec->fres_end)
+ }
+ if (cur + (offset_count * offset_size) > sec->fres_end) {
+ dbg_sec_uaccess("fre goes past end of subsection\n");
return -EFAULT;
+ }

fre->size = addr_size + 1 + (offset_count * offset_size);

@@ -184,8 +205,10 @@ static __always_inline int __read_fre(struct sframe_section *sec,

ra_off = sec->ra_off;
if (!ra_off) {
- if (!offset_count--)
+ if (!offset_count--) {
+ dbg_sec_uaccess("zero offset_count, can't find ra_off\n");
return -EFAULT;
+ }

UNSAFE_GET_USER_INC(ra_off, cur, offset_size, Efault);
}
@@ -196,8 +219,10 @@ static __always_inline int __read_fre(struct sframe_section *sec,
UNSAFE_GET_USER_INC(fp_off, cur, offset_size, Efault);
}

- if (offset_count)
+ if (offset_count) {
+ dbg_sec_uaccess("non-zero offset_count after reading fre\n");
return -EFAULT;
+ }

fre->ip_off = ip_off;
fre->cfa_off = cfa_off;
@@ -208,6 +233,7 @@ static __always_inline int __read_fre(struct sframe_section *sec,
return 0;

Efault:
+ dbg_sec_uaccess("fre usercopy failed\n");
return -EFAULT;
}

@@ -241,13 +267,20 @@ static __always_inline int __find_fre(struct sframe_section *sec,
which = !which;

ret = __read_fre(sec, fde, fre_addr, fre);
- if (ret)
+ if (ret) {
+ dbg_sec_uaccess("fde addr 0x%x: __read_fre(%u) failed\n",
+ fde->start_addr, i);
+ dbg_print_fde_uaccess(sec, fde);
return ret;
+ }

fre_addr += fre->size;

- if (prev_fre && fre->ip_off <= prev_fre->ip_off)
+ if (prev_fre && fre->ip_off <= prev_fre->ip_off) {
+ dbg_sec_uaccess("fde addr 0x%x: fre %u not sorted\n",
+ fde->start_addr, i);
return -EFAULT;
+ }

if (fre->ip_off > ip_off)
break;
diff --git a/kernel/unwind/sframe_debug.h b/kernel/unwind/sframe_debug.h
index 4d121cdbb760..3bb3c5574aee 100644
--- a/kernel/unwind/sframe_debug.h
+++ b/kernel/unwind/sframe_debug.h
@@ -13,6 +13,26 @@
#define dbg_sec(fmt, ...) \
dbg("%s: " fmt, sec->filename, ##__VA_ARGS__)

+#define __dbg_sec_descriptor(fmt, ...) \
+ __dynamic_pr_debug(&descriptor, "sframe: %s: " fmt, \
+ sec->filename, ##__VA_ARGS__)
+
+/*
+ * To avoid breaking uaccess rules, temporarily disable uaccess
+ * before calling printk.
+ */
+#define dbg_sec_uaccess(fmt, ...) \
+({ \
+ DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
+ if (DYNAMIC_DEBUG_BRANCH(descriptor)) { \
+ user_read_access_end(); \
+ __dbg_sec_descriptor(fmt, ##__VA_ARGS__); \
+ BUG_ON(!user_read_access_begin( \
+ (void __user *)sec->sframe_start, \
+ sec->sframe_end - sec->sframe_start)); \
+ } \
+})
+
static __always_inline void dbg_print_header(struct sframe_section *sec)
{
unsigned long fdes_end;
@@ -27,6 +47,15 @@ static __always_inline void dbg_print_header(struct sframe_section *sec)
sec->ra_off, sec->fp_off);
}

+static __always_inline void dbg_print_fde_uaccess(struct sframe_section *sec,
+ struct sframe_fde *fde)
+{
+ dbg_sec_uaccess("FDE: start_addr:0x%x func_size:0x%x "
+ "fres_off:0x%x fres_num:%d info:%u rep_size:%u\n",
+ fde->start_addr, fde->func_size,
+ fde->fres_off, fde->fres_num, fde->info, fde->rep_size);
+}
+
static inline void dbg_init(struct sframe_section *sec)
{
struct mm_struct *mm = current->mm;
@@ -53,8 +82,10 @@ static inline void dbg_free(struct sframe_section *sec)

#define dbg(args...) no_printk(args)
#define dbg_sec(args... ) no_printk(args)
+#define dbg_sec_uaccess(args...) no_printk(args)

static inline void dbg_print_header(struct sframe_section *sec) {}
+static inline void dbg_print_fde_uaccess(struct sframe_section *sec, struct sframe_fde *fde) {}

static inline void dbg_init(struct sframe_section *sec) {}
static inline void dbg_free(struct sframe_section *sec) {}
--
2.48.1