| Message ID | 20220627235512.2272783-1-quic_collinsd@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | spmi: trace: fix stack-out-of-bound access in SPMI tracing functions | expand |
On Mon, 27 Jun 2022 16:55:12 -0700 David Collins <quic_collinsd@quicinc.com> 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 thermal@2.0-ser/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 thermal@2.0-ser/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: stable@vger.kernel.org > Signed-off-by: David Collins <quic_collinsd@quicinc.com> > --- > 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) <rostedt@goodmis.org> -- 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 thermal@2.0-ser/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 thermal@2.0-ser/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: stable@vger.kernel.org > Signed-off-by: David Collins <quic_collinsd@quicinc.com> > --- Reviewed-by: Stephen Boyd <sboyd@kernel.org> 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 thermal@2.0-ser/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 thermal@2.0-ser/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: stable@vger.kernel.org > > Signed-off-by: David Collins <quic_collinsd@quicinc.com> > > --- > > Reviewed-by: Stephen Boyd <sboyd@kernel.org> > > Greg, can you pick this up directly? I don't have anything else for this > cycle. Now queued up, thanks. greg k-h
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]",