diff mbox series

spmi: trace: fix stack-out-of-bound access in SPMI tracing functions

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

Commit Message

David Collins June 27, 2022, 11:55 p.m. UTC
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(-)

Comments

Steven Rostedt July 8, 2022, 10:03 p.m. UTC | #1
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]",
Stephen Boyd July 24, 2022, 6:50 a.m. UTC | #2
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.
Greg KH July 24, 2022, 2:17 p.m. UTC | #3
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 mbox series

Patch

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]",