From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: UBSAN bug in hda_generic.c Date: Sat, 25 Jun 2016 13:28:00 +0200 Message-ID: References: <20160625110802.GA14579@localhost> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 27A63265134 for ; Sat, 25 Jun 2016 13:28:02 +0200 (CEST) In-Reply-To: <20160625110802.GA14579@localhost> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Bob Copeland Cc: alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org 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 > 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] [] dump_stack+0x86/0xd3 > [ 16.078317] [] ubsan_epilogue+0xd/0x40 > [ 16.078324] [] __ubsan_handle_out_of_bounds+0x67/0x70 > [ 16.078335] [] set_path_power+0x1bf/0x230 [snd_hda_codec_generic] > [ 16.078344] [] add_pin_power_ctls+0x8d/0xc0 [snd_hda_codec_generic] > [ 16.078352] [] ? pin_power_down_callback+0x20/0x20 [snd_hda_codec_generic] > [ 16.078360] [] add_all_pin_power_ctls+0x107/0x150 [snd_hda_codec_generic] > [ 16.078370] [] snd_hda_gen_parse_auto_config+0x2d73/0x49e0 [snd_hda_codec_generic] > [ 16.078376] [] ? trace_hardirqs_on_caller+0x1b0/0x2c0 > [ 16.078390] [] alc_parse_auto_config+0x147/0x310 [snd_hda_codec_realtek] > [ 16.078402] [] patch_alc269+0x23a/0x560 [snd_hda_codec_realtek] > [ 16.078417] [] hda_codec_driver_probe+0xa4/0x1a0 [snd_hda_codec] > [ 16.078424] [] driver_probe_device+0x101/0x380 > [ 16.078430] [] __driver_attach+0xb9/0x100 > [ 16.078438] [] ? driver_probe_device+0x380/0x380 > [ 16.078444] [] bus_for_each_dev+0x70/0xc0 > [ 16.078449] [] driver_attach+0x27/0x50 > [ 16.078454] [] bus_add_driver+0x166/0x2c0 > [ 16.078460] [] ? 0xffffffffa0369000 > [ 16.078465] [] driver_register+0x7d/0x130 > [ 16.078477] [] __hda_codec_driver_register+0x6f/0x90 [snd_hda_codec] > [ 16.078488] [] realtek_driver_init+0x1e/0x1000 [snd_hda_codec_realtek] > [ 16.078493] [] do_one_initcall+0x4e/0x1d0 > [ 16.078499] [] ? rcu_read_lock_sched_held+0x6d/0x80 > [ 16.078504] [] ? kmem_cache_alloc_trace+0x391/0x560 > [ 16.078510] [] ? do_init_module+0x28/0x273 > [ 16.078515] [] do_init_module+0x9b/0x273 > [ 16.078522] [] load_module+0x20b2/0x3410 > [ 16.078527] [] ? m_show+0x210/0x210 > [ 16.078533] [] ? kernel_read+0x66/0xe0 > [ 16.078541] [] SYSC_finit_module+0xba/0xc0 > [ 16.078547] [] SyS_finit_module+0xe/0x10 > [ 16.078552] [] entry_SYSCALL_64_fastpath+0x1f/0xbd > [ 16.078556] ================================================================================ > > Fix by checking path->depth before use. > > Signed-off-by: Bob Copeland > --- > 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/ >