All of lore.kernel.org
 help / color / mirror / Atom feed
* UBSAN bug in hda_generic.c
@ 2016-06-25 11:08 Bob Copeland
  2016-06-25 11:28 ` Takashi Iwai
  0 siblings, 1 reply; 3+ messages in thread
From: Bob Copeland @ 2016-06-25 11:08 UTC (permalink / raw)
  To: Takashi Iwai, Jaroslav Kysela; +Cc: alsa-devel

Hi,

I have UBSAN reporting an out-of-bounds array access (see below) on my
machine.  The following patch fixes the warning for me, but not sure if
that is just papering over some other bug.  Thanks in advance for looking!

>From 551e904f7a7aea9e9c03c439e554100643239c5c Mon Sep 17 00:00:00 2001
From: Bob Copeland <me@bobcopeland.com>
Date: Sat, 25 Jun 2016 06:53:44 -0400
Subject: [PATCH] ALSA: hda - fix read before array start

UBSAN reports the following warning from accessing path->path[-1]
in set_path_power():

[   16.078040] ================================================================================
[   16.078124] UBSAN: Undefined behaviour in sound/pci/hda/hda_generic.c:3981:17
[   16.078198] index -1 is out of range for type 'hda_nid_t [10]'
[   16.078270] CPU: 2 PID: 1738 Comm: modprobe Not tainted 4.7.0-rc1-wt+ #47
[   16.078274] Hardware name: LENOVO 3443CTO/3443CTO, BIOS G6ET23WW (1.02 ) 08/14/2012
[   16.078278]  ffff8800cb246000 ffff8800cb3638b8 ffffffff815c4fe3 0000000000000032
[   16.078286]  ffff8800cb3638e0 ffffffffffffffff ffff8800cb3638d0 ffffffff8162443d
[   16.078294]  ffffffffa0894200 ffff8800cb363920 ffffffff81624af7 0000000000000292
[   16.078302] Call Trace:
[   16.078311]  [<ffffffff815c4fe3>] dump_stack+0x86/0xd3
[   16.078317]  [<ffffffff8162443d>] ubsan_epilogue+0xd/0x40
[   16.078324]  [<ffffffff81624af7>] __ubsan_handle_out_of_bounds+0x67/0x70
[   16.078335]  [<ffffffffa087665f>] set_path_power+0x1bf/0x230 [snd_hda_codec_generic]
[   16.078344]  [<ffffffffa087880d>] add_pin_power_ctls+0x8d/0xc0 [snd_hda_codec_generic]
[   16.078352]  [<ffffffffa087f190>] ? pin_power_down_callback+0x20/0x20 [snd_hda_codec_generic]
[   16.078360]  [<ffffffffa0878947>] add_all_pin_power_ctls+0x107/0x150 [snd_hda_codec_generic]
[   16.078370]  [<ffffffffa08842b3>] snd_hda_gen_parse_auto_config+0x2d73/0x49e0 [snd_hda_codec_generic]
[   16.078376]  [<ffffffff81173360>] ? trace_hardirqs_on_caller+0x1b0/0x2c0
[   16.078390]  [<ffffffffa089df27>] alc_parse_auto_config+0x147/0x310 [snd_hda_codec_realtek]
[   16.078402]  [<ffffffffa08a332a>] patch_alc269+0x23a/0x560 [snd_hda_codec_realtek]
[   16.078417]  [<ffffffffa0838644>] hda_codec_driver_probe+0xa4/0x1a0 [snd_hda_codec]
[   16.078424]  [<ffffffff817bbac1>] driver_probe_device+0x101/0x380
[   16.078430]  [<ffffffff817bbdf9>] __driver_attach+0xb9/0x100
[   16.078438]  [<ffffffff817bbd40>] ? driver_probe_device+0x380/0x380
[   16.078444]  [<ffffffff817b8d20>] bus_for_each_dev+0x70/0xc0
[   16.078449]  [<ffffffff817bb087>] driver_attach+0x27/0x50
[   16.078454]  [<ffffffff817ba956>] bus_add_driver+0x166/0x2c0
[   16.078460]  [<ffffffffa0369000>] ? 0xffffffffa0369000
[   16.078465]  [<ffffffff817bd13d>] driver_register+0x7d/0x130
[   16.078477]  [<ffffffffa083816f>] __hda_codec_driver_register+0x6f/0x90 [snd_hda_codec]
[   16.078488]  [<ffffffffa036901e>] realtek_driver_init+0x1e/0x1000 [snd_hda_codec_realtek]
[   16.078493]  [<ffffffff8100215e>] do_one_initcall+0x4e/0x1d0
[   16.078499]  [<ffffffff8119f54d>] ? rcu_read_lock_sched_held+0x6d/0x80
[   16.078504]  [<ffffffff813701b1>] ? kmem_cache_alloc_trace+0x391/0x560
[   16.078510]  [<ffffffff812bb314>] ? do_init_module+0x28/0x273
[   16.078515]  [<ffffffff812bb387>] do_init_module+0x9b/0x273
[   16.078522]  [<ffffffff811e3782>] load_module+0x20b2/0x3410
[   16.078527]  [<ffffffff811df140>] ? m_show+0x210/0x210
[   16.078533]  [<ffffffff813b2b26>] ? kernel_read+0x66/0xe0
[   16.078541]  [<ffffffff811e4cfa>] SYSC_finit_module+0xba/0xc0
[   16.078547]  [<ffffffff811e4d1e>] SyS_finit_module+0xe/0x10
[   16.078552]  [<ffffffff81a860fc>] entry_SYSCALL_64_fastpath+0x1f/0xbd
[   16.078556] ================================================================================

Fix by checking path->depth before use.

Signed-off-by: Bob Copeland <me@bobcopeland.com>
---
 sound/pci/hda/hda_generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
index 320445f3bf73..bc23a9d8e7b3 100644
--- a/sound/pci/hda/hda_generic.c
+++ b/sound/pci/hda/hda_generic.c
@@ -3978,7 +3978,7 @@ static hda_nid_t set_path_power(struct hda_codec *codec, hda_nid_t nid,
 	for (n = 0; n < spec->paths.used; n++) {
 		path = snd_array_elem(&spec->paths, n);
 		if (path->path[0] == nid ||
-		    path->path[path->depth - 1] == nid) {
+		    (path->depth > 0 && path->path[path->depth - 1] == nid)) {
 			bool pin_old = path->pin_enabled;
 			bool stream_old = path->stream_enabled;
 
-- 
2.6.1



-- 
Bob Copeland %% http://bobcopeland.com/

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: UBSAN bug in hda_generic.c
  2016-06-25 11:08 UBSAN bug in hda_generic.c Bob Copeland
@ 2016-06-25 11:28 ` Takashi Iwai
  2016-06-25 11:52   ` Bob Copeland
  0 siblings, 1 reply; 3+ messages in thread
From: Takashi Iwai @ 2016-06-25 11:28 UTC (permalink / raw)
  To: Bob Copeland; +Cc: alsa-devel

On Sat, 25 Jun 2016 13:08:02 +0200,
Bob Copeland wrote:
> 
> Hi,
> 
> I have UBSAN reporting an out-of-bounds array access (see below) on my
> machine.  The following patch fixes the warning for me, but not sure if
> that is just papering over some other bug.  Thanks in advance for looking!

A better check would be to put
	if (!path->depth)
		continue;
before both path->path[0] and path->path[path->depth - 1]
evaluations.  path->depth=1 cannot exist, but path->depth=0 might be.

Could you resubmit with that fix?


thanks,

Takashi

>  		if (path->path[0] == nid ||
> -		    path->path[path->depth - 1] == nid) {

> 
> >From 551e904f7a7aea9e9c03c439e554100643239c5c Mon Sep 17 00:00:00 2001
> From: Bob Copeland <me@bobcopeland.com>
> Date: Sat, 25 Jun 2016 06:53:44 -0400
> Subject: [PATCH] ALSA: hda - fix read before array start
> 
> UBSAN reports the following warning from accessing path->path[-1]
> in set_path_power():
> 
> [   16.078040] ================================================================================
> [   16.078124] UBSAN: Undefined behaviour in sound/pci/hda/hda_generic.c:3981:17
> [   16.078198] index -1 is out of range for type 'hda_nid_t [10]'
> [   16.078270] CPU: 2 PID: 1738 Comm: modprobe Not tainted 4.7.0-rc1-wt+ #47
> [   16.078274] Hardware name: LENOVO 3443CTO/3443CTO, BIOS G6ET23WW (1.02 ) 08/14/2012
> [   16.078278]  ffff8800cb246000 ffff8800cb3638b8 ffffffff815c4fe3 0000000000000032
> [   16.078286]  ffff8800cb3638e0 ffffffffffffffff ffff8800cb3638d0 ffffffff8162443d
> [   16.078294]  ffffffffa0894200 ffff8800cb363920 ffffffff81624af7 0000000000000292
> [   16.078302] Call Trace:
> [   16.078311]  [<ffffffff815c4fe3>] dump_stack+0x86/0xd3
> [   16.078317]  [<ffffffff8162443d>] ubsan_epilogue+0xd/0x40
> [   16.078324]  [<ffffffff81624af7>] __ubsan_handle_out_of_bounds+0x67/0x70
> [   16.078335]  [<ffffffffa087665f>] set_path_power+0x1bf/0x230 [snd_hda_codec_generic]
> [   16.078344]  [<ffffffffa087880d>] add_pin_power_ctls+0x8d/0xc0 [snd_hda_codec_generic]
> [   16.078352]  [<ffffffffa087f190>] ? pin_power_down_callback+0x20/0x20 [snd_hda_codec_generic]
> [   16.078360]  [<ffffffffa0878947>] add_all_pin_power_ctls+0x107/0x150 [snd_hda_codec_generic]
> [   16.078370]  [<ffffffffa08842b3>] snd_hda_gen_parse_auto_config+0x2d73/0x49e0 [snd_hda_codec_generic]
> [   16.078376]  [<ffffffff81173360>] ? trace_hardirqs_on_caller+0x1b0/0x2c0
> [   16.078390]  [<ffffffffa089df27>] alc_parse_auto_config+0x147/0x310 [snd_hda_codec_realtek]
> [   16.078402]  [<ffffffffa08a332a>] patch_alc269+0x23a/0x560 [snd_hda_codec_realtek]
> [   16.078417]  [<ffffffffa0838644>] hda_codec_driver_probe+0xa4/0x1a0 [snd_hda_codec]
> [   16.078424]  [<ffffffff817bbac1>] driver_probe_device+0x101/0x380
> [   16.078430]  [<ffffffff817bbdf9>] __driver_attach+0xb9/0x100
> [   16.078438]  [<ffffffff817bbd40>] ? driver_probe_device+0x380/0x380
> [   16.078444]  [<ffffffff817b8d20>] bus_for_each_dev+0x70/0xc0
> [   16.078449]  [<ffffffff817bb087>] driver_attach+0x27/0x50
> [   16.078454]  [<ffffffff817ba956>] bus_add_driver+0x166/0x2c0
> [   16.078460]  [<ffffffffa0369000>] ? 0xffffffffa0369000
> [   16.078465]  [<ffffffff817bd13d>] driver_register+0x7d/0x130
> [   16.078477]  [<ffffffffa083816f>] __hda_codec_driver_register+0x6f/0x90 [snd_hda_codec]
> [   16.078488]  [<ffffffffa036901e>] realtek_driver_init+0x1e/0x1000 [snd_hda_codec_realtek]
> [   16.078493]  [<ffffffff8100215e>] do_one_initcall+0x4e/0x1d0
> [   16.078499]  [<ffffffff8119f54d>] ? rcu_read_lock_sched_held+0x6d/0x80
> [   16.078504]  [<ffffffff813701b1>] ? kmem_cache_alloc_trace+0x391/0x560
> [   16.078510]  [<ffffffff812bb314>] ? do_init_module+0x28/0x273
> [   16.078515]  [<ffffffff812bb387>] do_init_module+0x9b/0x273
> [   16.078522]  [<ffffffff811e3782>] load_module+0x20b2/0x3410
> [   16.078527]  [<ffffffff811df140>] ? m_show+0x210/0x210
> [   16.078533]  [<ffffffff813b2b26>] ? kernel_read+0x66/0xe0
> [   16.078541]  [<ffffffff811e4cfa>] SYSC_finit_module+0xba/0xc0
> [   16.078547]  [<ffffffff811e4d1e>] SyS_finit_module+0xe/0x10
> [   16.078552]  [<ffffffff81a860fc>] entry_SYSCALL_64_fastpath+0x1f/0xbd
> [   16.078556] ================================================================================
> 
> Fix by checking path->depth before use.
> 
> Signed-off-by: Bob Copeland <me@bobcopeland.com>
> ---
>  sound/pci/hda/hda_generic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
> index 320445f3bf73..bc23a9d8e7b3 100644
> --- a/sound/pci/hda/hda_generic.c
> +++ b/sound/pci/hda/hda_generic.c
> @@ -3978,7 +3978,7 @@ static hda_nid_t set_path_power(struct hda_codec *codec, hda_nid_t nid,
>  	for (n = 0; n < spec->paths.used; n++) {
>  		path = snd_array_elem(&spec->paths, n);
>  		if (path->path[0] == nid ||
> -		    path->path[path->depth - 1] == nid) {
> +		    (path->depth > 0 && path->path[path->depth - 1] == nid)) {
>  			bool pin_old = path->pin_enabled;
>  			bool stream_old = path->stream_enabled;
>  
> -- 
> 2.6.1
> 
> 
> 
> -- 
> Bob Copeland %% http://bobcopeland.com/
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: UBSAN bug in hda_generic.c
  2016-06-25 11:28 ` Takashi Iwai
@ 2016-06-25 11:52   ` Bob Copeland
  0 siblings, 0 replies; 3+ messages in thread
From: Bob Copeland @ 2016-06-25 11:52 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

On Sat, Jun 25, 2016 at 01:28:00PM +0200, Takashi Iwai wrote:
> On Sat, 25 Jun 2016 13:08:02 +0200,
> Bob Copeland wrote:
> > 
> > Hi,
> > 
> > I have UBSAN reporting an out-of-bounds array access (see below) on my
> > machine.  The following patch fixes the warning for me, but not sure if
> > that is just papering over some other bug.  Thanks in advance for looking!
> 
> A better check would be to put
> 	if (!path->depth)
> 		continue;
> before both path->path[0] and path->path[path->depth - 1]
> evaluations.  path->depth=1 cannot exist, but path->depth=0 might be.
> 
> Could you resubmit with that fix?

Sure, patch to follow in a separate mail so that patchwork can pick it up.

-- 
Bob Copeland %% http://bobcopeland.com/

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-06-25 11:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-25 11:08 UBSAN bug in hda_generic.c Bob Copeland
2016-06-25 11:28 ` Takashi Iwai
2016-06-25 11:52   ` Bob Copeland

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.