Traceprobe calculates dynamic string sizes before storing the values. A
probed pointer can change between those passes, so fetch_store_symstrlen()
can reserve space for a short symbol while fetch_store_symstring() observes
a longer one. The store path ignores the reserved length and allows the
second symbol to overwrite the trace record.
KASAN reports:
BUG: KASAN: slab-out-of-bounds in sized_strscpy
Write of size 8
sized_strscpy
module_address_lookup
__sprint_symbol
process_fetch_insn
Format the symbol into a temporary KSYM_SYMBOL_LEN buffer with
sprint_symbol(), then copy at most the reserved length minus the trailing
NUL. Using sprint_symbol() for both sizing and storage preserves identical
symbol formatting while bounding the destination write.
Fixes: b26a124cbfa8 ("tracing/probes: Add symstr type for dynamic events")
Signed-off-by: Yifan Zhang <[email protected]>
---
Changes in v2:
- use sprint_symbol() for both sizing and storage, with a bounded copy from
a temporary buffer, instead of formatting the destination with %pS
kernel/trace/trace_probe_tmpl.h | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h
index 8db12f758fdae..dea6772652550 100644
--- a/kernel/trace/trace_probe_tmpl.h
+++ b/kernel/trace/trace_probe_tmpl.h
@@ -87,15 +87,24 @@ fetch_store_symstrlen(unsigned long addr)
static nokprobe_inline int
fetch_store_symstring(unsigned long addr, void *dest, void *base)
{
+ char namebuf[KSYM_SYMBOL_LEN];
int maxlen = get_loc_len(*(u32 *)dest);
void *__dest;
+ int len;
if (unlikely(!maxlen))
return -ENOMEM;
__dest = get_loc_data(dest, base);
+ len = sprint_symbol(namebuf, addr);
+ if (len < 0)
+ return 0;
+
+ len = min(len, maxlen - 1);
+ memcpy(__dest, namebuf, len);
+ ((char *)__dest)[len] = '\0';
- return sprint_symbol(__dest, addr);
+ return len + 1;
}
/* common part of process_fetch_insn*/
--
2.39.5 (Apple Git-154)
