trace_spmi_write_begin() and trace_spmi_read_end() both call
memcpy() with a length of "len + 1". This leads to one extra
byte being read beyond the end of the specified buffer. Fix
this out-of-bound memory access by using a length of "len"
instead.
Here is a KASAN log showing the issue:
BUG: KASAN: stack-out-of-bounds in trace_event_raw_event_spmi_read_end+0x1d0/0x234
Read of size 2 at addr ffffffc0265b7540 by task [email protected]/1314
...
Call trace:
dump_backtrace+0x0/0x3e8
show_stack+0x2c/0x3c
dump_stack_lvl+0xdc/0x11c
print_address_description+0x74/0x384
kasan_report+0x188/0x268
kasan_check_range+0x270/0x2b0
memcpy+0x90/0xe8
trace_event_raw_event_spmi_read_end+0x1d0/0x234
spmi_read_cmd+0x294/0x3ac
spmi_ext_register_readl+0x84/0x9c
regmap_spmi_ext_read+0x144/0x1b0 [regmap_spmi]
_regmap_raw_read+0x40c/0x754
regmap_raw_read+0x3a0/0x514
regmap_bulk_read+0x418/0x494
adc5_gen3_poll_wait_hs+0xe8/0x1e0 [qcom_spmi_adc5_gen3]
...
__arm64_sys_read+0x4c/0x60
invoke_syscall+0x80/0x218
el0_svc_common+0xec/0x1c8
...
addr ffffffc0265b7540 is located in stack of task [email protected]/1314 at offset 32 in frame:
adc5_gen3_poll_wait_hs+0x0/0x1e0 [qcom_spmi_adc5_gen3]
this frame has 1 object:
[32, 33) 'status'
Memory state around the buggy address:
ffffffc0265b7400: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
ffffffc0265b7480: 04 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
>ffffffc0265b7500: 00 00 00 00 f1 f1 f1 f1 01 f3 f3 f3 00 00 00 00
^
ffffffc0265b7580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffffffc0265b7600: f1 f1 f1 f1 01 f2 07 f2 f2 f2 01 f3 00 00 00 00
==================================================================
Fixes: a9fce374815d ("spmi: add command tracepoints for SPMI")
Cc: [email protected]
Signed-off-by: David Collins <[email protected]>
---
include/trace/events/spmi.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/trace/events/spmi.h b/include/trace/events/spmi.h
index 8b60efe18ba6..a6819fd85cdf 100644
--- a/include/trace/events/spmi.h
+++ b/include/trace/events/spmi.h
@@ -21,15 +21,15 @@ TRACE_EVENT(spmi_write_begin,
__field ( u8, sid )
__field ( u16, addr )
__field ( u8, len )
- __dynamic_array ( u8, buf, len + 1 )
+ __dynamic_array ( u8, buf, len )
),
TP_fast_assign(
__entry->opcode = opcode;
__entry->sid = sid;
__entry->addr = addr;
- __entry->len = len + 1;
- memcpy(__get_dynamic_array(buf), buf, len + 1);
+ __entry->len = len;
+ memcpy(__get_dynamic_array(buf), buf, len);
),
TP_printk("opc=%d sid=%02d addr=0x%04x len=%d buf=0x[%*phD]",
@@ -92,7 +92,7 @@ TRACE_EVENT(spmi_read_end,
__field ( u16, addr )
__field ( int, ret )
__field ( u8, len )
- __dynamic_array ( u8, buf, len + 1 )
+ __dynamic_array ( u8, buf, len )
),
TP_fast_assign(
@@ -100,8 +100,8 @@ TRACE_EVENT(spmi_read_end,
__entry->sid = sid;
__entry->addr = addr;
__entry->ret = ret;
- __entry->len = len + 1;
- memcpy(__get_dynamic_array(buf), buf, len + 1);
+ __entry->len = len;
+ memcpy(__get_dynamic_array(buf), buf, len);
),
TP_printk("opc=%d sid=%02d addr=0x%04x ret=%d len=%02d buf=0x[%*phD]",
--
2.25.1
On Mon, 27 Jun 2022 16:55:12 -0700
David Collins <[email protected]> wrote:
> trace_spmi_write_begin() and trace_spmi_read_end() both call
> memcpy() with a length of "len + 1". This leads to one extra
> byte being read beyond the end of the specified buffer. Fix
> this out-of-bound memory access by using a length of "len"
> instead.
>
> Here is a KASAN log showing the issue:
>
> BUG: KASAN: stack-out-of-bounds in trace_event_raw_event_spmi_read_end+0x1d0/0x234
> Read of size 2 at addr ffffffc0265b7540 by task [email protected]/1314
> ...
> Call trace:
> dump_backtrace+0x0/0x3e8
> show_stack+0x2c/0x3c
> dump_stack_lvl+0xdc/0x11c
> print_address_description+0x74/0x384
> kasan_report+0x188/0x268
> kasan_check_range+0x270/0x2b0
> memcpy+0x90/0xe8
> trace_event_raw_event_spmi_read_end+0x1d0/0x234
> spmi_read_cmd+0x294/0x3ac
> spmi_ext_register_readl+0x84/0x9c
> regmap_spmi_ext_read+0x144/0x1b0 [regmap_spmi]
> _regmap_raw_read+0x40c/0x754
> regmap_raw_read+0x3a0/0x514
> regmap_bulk_read+0x418/0x494
> adc5_gen3_poll_wait_hs+0xe8/0x1e0 [qcom_spmi_adc5_gen3]
> ...
> __arm64_sys_read+0x4c/0x60
> invoke_syscall+0x80/0x218
> el0_svc_common+0xec/0x1c8
> ...
>
> addr ffffffc0265b7540 is located in stack of task [email protected]/1314 at offset 32 in frame:
> adc5_gen3_poll_wait_hs+0x0/0x1e0 [qcom_spmi_adc5_gen3]
>
> this frame has 1 object:
> [32, 33) 'status'
>
> Memory state around the buggy address:
> ffffffc0265b7400: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
> ffffffc0265b7480: 04 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
> >ffffffc0265b7500: 00 00 00 00 f1 f1 f1 f1 01 f3 f3 f3 00 00 00 00
> ^
> ffffffc0265b7580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffffffc0265b7600: f1 f1 f1 f1 01 f2 07 f2 f2 f2 01 f3 00 00 00 00
> ==================================================================
>
> Fixes: a9fce374815d ("spmi: add command tracepoints for SPMI")
> Cc: [email protected]
> Signed-off-by: David Collins <[email protected]>
> ---
> include/trace/events/spmi.h | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/trace/events/spmi.h b/include/trace/events/spmi.h
> index 8b60efe18ba6..a6819fd85cdf 100644
> --- a/include/trace/events/spmi.h
> +++ b/include/trace/events/spmi.h
> @@ -21,15 +21,15 @@ TRACE_EVENT(spmi_write_begin,
> __field ( u8, sid )
> __field ( u16, addr )
> __field ( u8, len )
> - __dynamic_array ( u8, buf, len + 1 )
> + __dynamic_array ( u8, buf, len )
> ),
>
> TP_fast_assign(
> __entry->opcode = opcode;
> __entry->sid = sid;
> __entry->addr = addr;
> - __entry->len = len + 1;
> - memcpy(__get_dynamic_array(buf), buf, len + 1);
> + __entry->len = len;
> + memcpy(__get_dynamic_array(buf), buf, len);
> ),
>
> TP_printk("opc=%d sid=%02d addr=0x%04x len=%d buf=0x[%*phD]",
> @@ -92,7 +92,7 @@ TRACE_EVENT(spmi_read_end,
> __field ( u16, addr )
> __field ( int, ret )
> __field ( u8, len )
> - __dynamic_array ( u8, buf, len + 1 )
> + __dynamic_array ( u8, buf, len )
> ),
>
> TP_fast_assign(
> @@ -100,8 +100,8 @@ TRACE_EVENT(spmi_read_end,
> __entry->sid = sid;
> __entry->addr = addr;
> __entry->ret = ret;
> - __entry->len = len + 1;
> - memcpy(__get_dynamic_array(buf), buf, len + 1);
> + __entry->len = len;
> + memcpy(__get_dynamic_array(buf), buf, len);
> ),
Looks legit,
Acked-by: Steven Rostedt (Google) <[email protected]>
-- Steve
>
> TP_printk("opc=%d sid=%02d addr=0x%04x ret=%d len=%02d buf=0x[%*phD]",
Quoting David Collins (2022-06-27 16:55:12)
> trace_spmi_write_begin() and trace_spmi_read_end() both call
> memcpy() with a length of "len + 1". This leads to one extra
> byte being read beyond the end of the specified buffer. Fix
> this out-of-bound memory access by using a length of "len"
> instead.
>
> Here is a KASAN log showing the issue:
>
> BUG: KASAN: stack-out-of-bounds in trace_event_raw_event_spmi_read_end+0x1d0/0x234
> Read of size 2 at addr ffffffc0265b7540 by task [email protected]/1314
> ...
> Call trace:
> dump_backtrace+0x0/0x3e8
> show_stack+0x2c/0x3c
> dump_stack_lvl+0xdc/0x11c
> print_address_description+0x74/0x384
> kasan_report+0x188/0x268
> kasan_check_range+0x270/0x2b0
> memcpy+0x90/0xe8
> trace_event_raw_event_spmi_read_end+0x1d0/0x234
> spmi_read_cmd+0x294/0x3ac
> spmi_ext_register_readl+0x84/0x9c
> regmap_spmi_ext_read+0x144/0x1b0 [regmap_spmi]
> _regmap_raw_read+0x40c/0x754
> regmap_raw_read+0x3a0/0x514
> regmap_bulk_read+0x418/0x494
> adc5_gen3_poll_wait_hs+0xe8/0x1e0 [qcom_spmi_adc5_gen3]
> ...
> __arm64_sys_read+0x4c/0x60
> invoke_syscall+0x80/0x218
> el0_svc_common+0xec/0x1c8
> ...
>
> addr ffffffc0265b7540 is located in stack of task [email protected]/1314 at offset 32 in frame:
> adc5_gen3_poll_wait_hs+0x0/0x1e0 [qcom_spmi_adc5_gen3]
>
> this frame has 1 object:
> [32, 33) 'status'
>
> Memory state around the buggy address:
> ffffffc0265b7400: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
> ffffffc0265b7480: 04 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
> >ffffffc0265b7500: 00 00 00 00 f1 f1 f1 f1 01 f3 f3 f3 00 00 00 00
> ^
> ffffffc0265b7580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffffffc0265b7600: f1 f1 f1 f1 01 f2 07 f2 f2 f2 01 f3 00 00 00 00
> ==================================================================
>
> Fixes: a9fce374815d ("spmi: add command tracepoints for SPMI")
> Cc: [email protected]
> Signed-off-by: David Collins <[email protected]>
> ---
Reviewed-by: Stephen Boyd <[email protected]>
Greg, can you pick this up directly? I don't have anything else for this
cycle.
On Sat, Jul 23, 2022 at 11:50:50PM -0700, Stephen Boyd wrote:
> Quoting David Collins (2022-06-27 16:55:12)
> > trace_spmi_write_begin() and trace_spmi_read_end() both call
> > memcpy() with a length of "len + 1". This leads to one extra
> > byte being read beyond the end of the specified buffer. Fix
> > this out-of-bound memory access by using a length of "len"
> > instead.
> >
> > Here is a KASAN log showing the issue:
> >
> > BUG: KASAN: stack-out-of-bounds in trace_event_raw_event_spmi_read_end+0x1d0/0x234
> > Read of size 2 at addr ffffffc0265b7540 by task [email protected]/1314
> > ...
> > Call trace:
> > dump_backtrace+0x0/0x3e8
> > show_stack+0x2c/0x3c
> > dump_stack_lvl+0xdc/0x11c
> > print_address_description+0x74/0x384
> > kasan_report+0x188/0x268
> > kasan_check_range+0x270/0x2b0
> > memcpy+0x90/0xe8
> > trace_event_raw_event_spmi_read_end+0x1d0/0x234
> > spmi_read_cmd+0x294/0x3ac
> > spmi_ext_register_readl+0x84/0x9c
> > regmap_spmi_ext_read+0x144/0x1b0 [regmap_spmi]
> > _regmap_raw_read+0x40c/0x754
> > regmap_raw_read+0x3a0/0x514
> > regmap_bulk_read+0x418/0x494
> > adc5_gen3_poll_wait_hs+0xe8/0x1e0 [qcom_spmi_adc5_gen3]
> > ...
> > __arm64_sys_read+0x4c/0x60
> > invoke_syscall+0x80/0x218
> > el0_svc_common+0xec/0x1c8
> > ...
> >
> > addr ffffffc0265b7540 is located in stack of task [email protected]/1314 at offset 32 in frame:
> > adc5_gen3_poll_wait_hs+0x0/0x1e0 [qcom_spmi_adc5_gen3]
> >
> > this frame has 1 object:
> > [32, 33) 'status'
> >
> > Memory state around the buggy address:
> > ffffffc0265b7400: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
> > ffffffc0265b7480: 04 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
> > >ffffffc0265b7500: 00 00 00 00 f1 f1 f1 f1 01 f3 f3 f3 00 00 00 00
> > ^
> > ffffffc0265b7580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > ffffffc0265b7600: f1 f1 f1 f1 01 f2 07 f2 f2 f2 01 f3 00 00 00 00
> > ==================================================================
> >
> > Fixes: a9fce374815d ("spmi: add command tracepoints for SPMI")
> > Cc: [email protected]
> > Signed-off-by: David Collins <[email protected]>
> > ---
>
> Reviewed-by: Stephen Boyd <[email protected]>
>
> Greg, can you pick this up directly? I don't have anything else for this
> cycle.
Now queued up, thanks.
greg k-h