Re: PROBLEM: hda-intel divide by zero kernel crash in azx_position_ok()

[Takashi Iwai <tiwai@suse.de>]

At Fri, 05 Feb 2010 18:32:14 -0500,
Jody@tritech wrote:
> 
> On the latest kernel(s) my audio crashes the kernel. Mp3blaster
> triggers the problem most easily. As I am filing this from a mobile
> device I cannot directly attach the full report.  Instead I have
> generated and uploaded it.  Please let me know if I have omitted any
> needed information via this email address.
> I have stored the full bug report at
> http://nctritech.net/bugreport.txt

Thanks for reporting.
(But it'd have been better if you just post with the problem
 description in the mail...)

I cite from the above:
> Full description of the problem/report:
> Using mp3blaster-3.2.5 (latest version) to play MP3 audio, I am able to
> crash the kernel by stopping and restarting playback using the "5" key
> repeatedly.  This happens as a normal user, not only as root.  Kernel
> backtrace points to azx_position_ok() dividing by zero, so wrote a tiny
> patch to investigate which reported via printk() values of pos and
> azx_dev->period_bytes; on crash, both were 0.  The offending operation
> does: if (pos % azx_dev->period_bytes > azx_dev->period_bytes / 2)
> which obviously is the source of the crash.
> This happens on linux 2.6.32.7 as well as linux 2.6.33-rc6.
...
> --- linux-2.6.33-rc6/sound/pci/hda/hda_intel.c	  2010-01-28 18:06:20.000000000 -0500
> +++ linux-2.6.33-rc6-work/sound/pci/hda/hda_intel.c	     2010-02-05 15:55:48.116620701 -0500
> @@ -1858,6 +1878,12 @@
>  
>	if (!bdl_pos_adj[chip->dev_index])
> 	   return 1; /* no delayed ack */
> +	   if (azx_dev->period_bytes == 0) {
> +	      printk(KERN_WARNING
> +			"hda-intel: Divide by zero was avoided "
> +		    	   "in azx_dev->period_bytes.\n");
> +		       return 0;
> +	       }
>       if (pos % azx_dev->period_bytes > azx_dev->period_bytes / 2)
> 	       	  return 0; /* NG - it's below the period boundary */
> 	return 1; /* OK, it's fine */

It's weird because the place there shouldn't be reached when the
running flag is cleared.  And period_bytes = 0 is only before the
stream preparation of after the hw_free call, thus running flag is
certainly off.  Moreover, these setups are protected by spinlock, thus
races are quite unlikely.  So, I suspect this is a false IRQ and stale 
register bit on your hardware.

Of course, your fix is fine and harmless for normal systems, thus I'd
like to put it in.

Could you repost the patch with a changelog and your sign-off for
merging to the upstream?


thanks,

Takashi