From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965690AbcIHPsm (ORCPT ); Thu, 8 Sep 2016 11:48:42 -0400 Received: from mx2.suse.de ([195.135.220.15]:47210 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965210AbcIHPsl (ORCPT ); Thu, 8 Sep 2016 11:48:41 -0400 Date: Thu, 08 Sep 2016 17:48:39 +0200 Message-ID: From: Takashi Iwai To: Dmitry Vyukov Cc: Vegard Nossum , Vegard Nossum , Jaroslav Kysela , Kangjie Lu , alsa-devel@alsa-project.org, LKML , syzkaller Subject: Re: sound: divide by 0 in snd_hrtimer_callback (or hang) In-Reply-To: References:

User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.5 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 08 Sep 2016 17:14:18 +0200, Dmitry Vyukov wrote: > > On Wed, Sep 7, 2016 at 5:00 PM, Takashi Iwai wrote: > > On Tue, 06 Sep 2016 18:10:30 +0200, > > Takashi Iwai wrote: > >> > >> On Tue, 06 Sep 2016 16:06:08 +0200, > >> Dmitry Vyukov wrote: > >> > > >> > > Do you really see the zero-division error with sticks=1 fix? That's > >> > > unexpected. > >> > > >> > Yes, just reproduced it again. > >> > I am on 0693c28cfc8e25f18c25b65a8942c026f1854a3c of linux-next now, so > >> > I already have: > >> > commit 6b760bb2c63a9e322c0e4a0b5daf335ad93d5a33 > >> > ALSA: timer: fix division by zero after SNDRV_TIMER_IOCTL_CONTINUE > >> > > >> > Compiled the program verbatim and run it using stress utility (http > >> > s://godoc.org/golang.org/x/tools/cmd/stress namely just runs the > >> > program in parallel loop) and instantly get the following crash. > >> > >> OK, I'll take a deeper look. > > > > The fix patch below seems working on my system. Please give it a > > try. > > > Pulled this from linux-next. > This should fix both division and stalls, right? Yes, it looks so. The latter one seems to be the side-effect of the stuck by the first one. > I will let you know if I see any of these again. Thanks. Takashi > > > > -- 8< -- > > From: Takashi Iwai > > Subject: [PATCH] ALSA: timer: Fix zero-division by continue of uninitialized > > instance > > > > When a user timer instance is continued without the explicit start > > beforehand, the system gets eventually zero-division error like: > > > > divide error: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN > > CPU: 1 PID: 27320 Comm: syz-executor Not tainted 4.8.0-rc3-next-20160825+ #8 > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > > task: ffff88003c9b2280 task.stack: ffff880027280000 > > RIP: 0010:[] [< inline >] ktime_divns include/linux/ktime.h:195 > > RIP: 0010:[] [] snd_hrtimer_callback+0x1bc/0x3c0 sound/core/hrtimer.c:62 > > Call Trace: > > > > [< inline >] __run_hrtimer kernel/time/hrtimer.c:1238 > > [] __hrtimer_run_queues+0x325/0xe70 kernel/time/hrtimer.c:1302 > > [] hrtimer_interrupt+0x18b/0x420 kernel/time/hrtimer.c:1336 > > [] local_apic_timer_interrupt+0x6f/0xe0 arch/x86/kernel/apic/apic.c:933 > > [] smp_apic_timer_interrupt+0x76/0xa0 arch/x86/kernel/apic/apic.c:957 > > [] apic_timer_interrupt+0x8c/0xa0 arch/x86/entry/entry_64.S:487 > > > > ..... > > > > Although a similar issue was spotted and a fix patch was merged in > > commit [6b760bb2c63a: ALSA: timer: fix division by zero after > > SNDRV_TIMER_IOCTL_CONTINUE], it seems covering only a part of > > iceberg. > > > > In this patch, we fix the issue a bit more drastically. Basically the > > continue of an uninitialized timer is supposed to be a fresh start, so > > we do it for user timers. For the direct snd_timer_continue() call, > > there is no way to pass the initial tick value, so we kick out for the > > uninitialized case. > > > > Reported-by: Reported-by: Dmitry Vyukov > > Cc: > > Signed-off-by: Takashi Iwai > > --- > > sound/core/timer.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/sound/core/timer.c b/sound/core/timer.c > > index 2706061fc1ea..fc144f43faa6 100644 > > --- a/sound/core/timer.c > > +++ b/sound/core/timer.c > > @@ -35,6 +35,9 @@ > > #include > > #include > > > > +/* internal flags */ > > +#define SNDRV_TIMER_IFLG_PAUSED 0x00010000 > > + > > #if IS_ENABLED(CONFIG_SND_HRTIMER) > > #define DEFAULT_TIMER_LIMIT 4 > > #else > > @@ -539,6 +542,10 @@ static int snd_timer_stop1(struct snd_timer_instance *timeri, bool stop) > > } > > } > > timeri->flags &= ~(SNDRV_TIMER_IFLG_RUNNING | SNDRV_TIMER_IFLG_START); > > + if (stop) > > + timeri->flags &= ~SNDRV_TIMER_IFLG_PAUSED; > > + else > > + timeri->flags |= SNDRV_TIMER_IFLG_PAUSED; > > snd_timer_notify1(timeri, stop ? SNDRV_TIMER_EVENT_STOP : > > SNDRV_TIMER_EVENT_CONTINUE); > > unlock: > > @@ -600,6 +607,10 @@ int snd_timer_stop(struct snd_timer_instance *timeri) > > */ > > int snd_timer_continue(struct snd_timer_instance *timeri) > > { > > + /* timer can continue only after pause */ > > + if (!(timeri->flags & SNDRV_TIMER_IFLG_PAUSED)) > > + return -EINVAL; > > + > > if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE) > > return snd_timer_start_slave(timeri, false); > > else > > @@ -1831,6 +1842,9 @@ static int snd_timer_user_continue(struct file *file) > > tu = file->private_data; > > if (!tu->timeri) > > return -EBADFD; > > + /* start timer instead of continue if it's not used before */ > > + if (!(tu->timeri->flags & SNDRV_TIMER_IFLG_PAUSED)) > > + return snd_timer_user_start(file); > > tu->timeri->lost = 0; > > return (err = snd_timer_continue(tu->timeri)) < 0 ? err : 0; > > } > > -- > > 2.10.0 > > >