From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: sb_mixer.c/save_mixer() awfully buggy? Date: Mon, 23 Jun 2008 12:46:07 +0200 Message-ID: References: <20080622224635.GA2431@rhlx01.hs-esslingen.de> 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 (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 2F00910385D for ; Mon, 23 Jun 2008 12:46:08 +0200 (CEST) In-Reply-To: <20080622224635.GA2431@rhlx01.hs-esslingen.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: alsa-devel-bounces@alsa-project.org Errors-To: alsa-devel-bounces@alsa-project.org To: Andreas Mohr Cc: alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org At Mon, 23 Jun 2008 00:46:35 +0200, Andreas Mohr wrote: > > Hi all, > > with my ALS4000, I'm hitting this upon resume: > > ALSA sound/isa/sb/sb_mixer.c:936: BUG? (num_regs > (sizeof(chip->saved_regs) / sizeof( > (chip->saved_regs)[0]) + (sizeof(char[1 - 2 * !!(__builtin_types_compatible_p(typeof(c > hip->saved_regs), typeof(&chip->saved_regs[0])))]) - 1))) > Pid: 4498, comm: my_sleep Not tainted 2.6.26-rc3 #1 > [] restore_mixer+0x6c/0x70 [snd_sb_common] > [] snd_sbmixer_resume+0x77/0x80 [snd_sb_common] > [] snd_als4000_resume+0x5a/0xd0 [snd_als4000] > [] pci_device_resume+0x22/0x60 > [] device_resume+0x86/0x100 > [] hibernation_snapshot+0xc7/0x180 > [] ? freeze_processes+0x3f/0x80 > [] hibernate+0xdc/0x180 > [] ? state_store+0x0/0xd0 > [] state_store+0xbf/0xd0 > [] ? state_store+0x0/0xd0 > [] kobj_attr_store+0x24/0x30 > [] sysfs_write_file+0xbb/0x110 > [] vfs_write+0x96/0x130 > [] ? sysfs_write_file+0x0/0x110 > [] sys_write+0x3d/0x70 > [] syscall_call+0x7/0xb > ======================= > > triggered by this line: > > static void restore_mixer(struct snd_sb *chip, unsigned char *regs, int num_regs) > { > unsigned char *val = chip->saved_regs; > snd_assert(num_regs > ARRAY_SIZE(chip->saved_regs), return); > for (; num_regs; num_regs--) > snd_sbmixer_write(chip, *regs++, *val++); > } > > Uhmm, forgive my utterly dumb state of mind, but isn't this assert > _exactly_ wrong? > We assert that the number of regs to copy is _higher_ than the array > size of the content we want to copy, IOW we want to make _certain_ > that we'll hit a nice array overflow or segmentation violation later. > Doesn't make too much - well, exactly zero, zilch - sense to me. > > Dito save_mixer(). Indeed, it's a silly bug. It just be num_regs <= ARRAY_SIZE(). The codepath wasn't tested with the debug environment. I fixed it on my tree now. > > Forgive me if this is already fixed in ALSA head (still exists in current > 2.6.26-rc7). > Sounds like 2.6.26(!) material to me if it is indeed buggy. Yeah, will do a pull request. thanks, Takashi