From 6639484ddaf6707b41082c9fa9ca9af342df6402 Mon Sep 17 00:00:00 2001 From: Libin Yang Date: Fri, 29 Jan 2016 20:39:09 +0800 Subject: ALSA: hda - disable dynamic clock gating on Broxton before reset On Broxton, to make sure the reset controller works properly, MISCBDCGE bit (bit 6) in CGCTL (0x48) of PCI configuration space need be cleared before reset and set back to 1 after reset. Otherwise, it may prevent the CORB/RIRB logic from being reset. Signed-off-by: Libin Yang Cc: # v4.4+ Signed-off-by: Takashi Iwai --- sound/pci/hda/hda_intel.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'sound') diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 256e6cda218f..4045dca3d699 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -90,6 +90,8 @@ enum { #define NVIDIA_HDA_ENABLE_COHBIT 0x01 /* Defines for Intel SCH HDA snoop control */ +#define INTEL_HDA_CGCTL 0x48 +#define INTEL_HDA_CGCTL_MISCBDCGE (0x1 << 6) #define INTEL_SCH_HDA_DEVC 0x78 #define INTEL_SCH_HDA_DEVC_NOSNOOP (0x1<<11) @@ -534,10 +536,21 @@ static void hda_intel_init_chip(struct azx *chip, bool full_reset) { struct hdac_bus *bus = azx_bus(chip); struct pci_dev *pci = chip->pci; + u32 val; if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) snd_hdac_set_codec_wakeup(bus, true); + if (IS_BROXTON(pci)) { + pci_read_config_dword(pci, INTEL_HDA_CGCTL, &val); + val = val & ~INTEL_HDA_CGCTL_MISCBDCGE; + pci_write_config_dword(pci, INTEL_HDA_CGCTL, val); + } azx_init_chip(chip, full_reset); + if (IS_BROXTON(pci)) { + pci_read_config_dword(pci, INTEL_HDA_CGCTL, &val); + val = val | INTEL_HDA_CGCTL_MISCBDCGE; + pci_write_config_dword(pci, INTEL_HDA_CGCTL, val); + } if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) snd_hdac_set_codec_wakeup(bus, false); -- cgit v1.2.3 From 5327d6ba975042fd3da50ac6e94d1e9551ebeaec Mon Sep 17 00:00:00 2001 From: Jurgen Kramer Date: Fri, 29 Jan 2016 14:49:55 +0100 Subject: ALSA: usb-audio: Fix OPPO HA-1 vendor ID In my patch adding native DSD support for the Oppo HA-1, the wrong vendor ID got through. This patch fixes the vendor ID and aligns the comment. Fixes: a4eae3a506ea ('ALSA: usb: Add native DSD support for Oppo HA-1') Signed-off-by: Jurgen Kramer Cc: Signed-off-by: Takashi Iwai --- sound/usb/quirks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sound') diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index a75d9ce7d77a..62e677dd8654 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -1281,7 +1281,7 @@ u64 snd_usb_interface_dsd_format_quirks(struct snd_usb_audio *chip, case USB_ID(0x20b1, 0x3008): /* iFi Audio micro/nano iDSD */ case USB_ID(0x20b1, 0x2008): /* Matrix Audio X-Sabre */ case USB_ID(0x20b1, 0x300a): /* Matrix Audio Mini-i Pro */ - case USB_ID(0x22d8, 0x0416): /* OPPO HA-1*/ + case USB_ID(0x22d9, 0x0416): /* OPPO HA-1 */ if (fp->altsetting == 2) return SNDRV_PCM_FMTBIT_DSD_U32_BE; break; -- cgit v1.2.3 From ad678b4ccd41aa51cf5f142c0e8cffe9d61fc2bf Mon Sep 17 00:00:00 2001 From: Jurgen Kramer Date: Fri, 29 Jan 2016 14:59:25 +0100 Subject: ALSA: usb-audio: Add native DSD support for PS Audio NuWave DAC This patch adds native DSD support for the PS Audio NuWave DAC. Signed-off-by: Jurgen Kramer Cc: Signed-off-by: Takashi Iwai --- sound/usb/quirks.c | 1 + 1 file changed, 1 insertion(+) (limited to 'sound') diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index 62e677dd8654..75f0e26514e7 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -1290,6 +1290,7 @@ u64 snd_usb_interface_dsd_format_quirks(struct snd_usb_audio *chip, case USB_ID(0x20b1, 0x2009): /* DIYINHK DSD DXD 384kHz USB to I2S/DSD */ case USB_ID(0x20b1, 0x2023): /* JLsounds I2SoverUSB */ case USB_ID(0x20b1, 0x3023): /* Aune X1S 32BIT/384 DSD DAC */ + case USB_ID(0x2616, 0x0106): /* PS Audio NuWave DAC */ if (fp->altsetting == 3) return SNDRV_PCM_FMTBIT_DSD_U32_BE; break; -- cgit v1.2.3 From 1b3c993a699bed282e47c3f7c49d539c331dae04 Mon Sep 17 00:00:00 2001 From: Lev Lybin Date: Fri, 29 Jan 2016 22:55:11 +0700 Subject: ALSA: usb-audio: Add quirk for Microsoft LifeCam HD-6000 Microsoft LifeCam HD-6000 (045e:076f) requires the similar quirk for avoiding the stall due to the invalid sample rate reads. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=111491 Signed-off-by: Lev Lybin Cc: Signed-off-by: Takashi Iwai --- sound/usb/quirks.c | 1 + 1 file changed, 1 insertion(+) (limited to 'sound') diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index 75f0e26514e7..4f6ce1cac8e2 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -1121,6 +1121,7 @@ bool snd_usb_get_sample_rate_quirk(struct snd_usb_audio *chip) switch (chip->usb_id) { case USB_ID(0x045E, 0x075D): /* MS Lifecam Cinema */ case USB_ID(0x045E, 0x076D): /* MS Lifecam HD-5000 */ + case USB_ID(0x045E, 0x076F): /* MS Lifecam HD-6000 */ case USB_ID(0x045E, 0x0772): /* MS Lifecam Studio */ case USB_ID(0x045E, 0x0779): /* MS Lifecam HD-3000 */ case USB_ID(0x04D8, 0xFEEA): /* Benchmark DAC1 Pre */ -- cgit v1.2.3 From f1d51595a2a54d725cd6a21dd54508335a14ab90 Mon Sep 17 00:00:00 2001 From: Insu Yun Date: Fri, 29 Jan 2016 10:56:11 -0500 Subject: ALSA: emu10k1: correctly handling failed thread creation Since kthread_create can be failed, it needs to check whether error occurred and return error code. Signed-off-by: Insu Yun Signed-off-by: Takashi Iwai --- sound/pci/emu10k1/emu10k1_main.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'sound') diff --git a/sound/pci/emu10k1/emu10k1_main.c b/sound/pci/emu10k1/emu10k1_main.c index 28e2f8b42f5e..891453451543 100644 --- a/sound/pci/emu10k1/emu10k1_main.c +++ b/sound/pci/emu10k1/emu10k1_main.c @@ -1141,6 +1141,14 @@ static int snd_emu10k1_emu1010_init(struct snd_emu10k1 *emu) emu->emu1010.firmware_thread = kthread_create(emu1010_firmware_thread, emu, "emu1010_firmware"); + if (IS_ERR(emu->emu1010.firmware_thread)) { + err = PTR_ERR(emu->emu1010.firmware_thread); + emu->emu1010.firmware_thread = NULL; + dev_info(emu->card->dev, + "emu1010: Creating thread failed\n"); + return err; + } + wake_up_process(emu->emu1010.firmware_thread); } -- cgit v1.2.3 From 2d1b5c08366acd46c35a2e9aba5d650cb5bf5c19 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Mon, 1 Feb 2016 12:06:42 +0100 Subject: ALSA: seq: Fix race at closing in virmidi driver The virmidi driver has an open race at closing its assigned rawmidi device, and this may lead to use-after-free in snd_seq_deliver_single_event(). Plug the hole by properly protecting the linked list deletion and calling in the right order in snd_virmidi_input_close(). BugLink: http://lkml.kernel.org/r/CACT4Y+Zd66+w12fNN85-425cVQT=K23kWbhnCEcMB8s3us-Frw@mail.gmail.com Reported-by: Dmitry Vyukov Tested-by: Dmitry Vyukov Cc: Signed-off-by: Takashi Iwai --- sound/core/seq/seq_virmidi.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'sound') diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c index 3da2d48610b3..f71aedfb408c 100644 --- a/sound/core/seq/seq_virmidi.c +++ b/sound/core/seq/seq_virmidi.c @@ -254,9 +254,13 @@ static int snd_virmidi_output_open(struct snd_rawmidi_substream *substream) */ static int snd_virmidi_input_close(struct snd_rawmidi_substream *substream) { + struct snd_virmidi_dev *rdev = substream->rmidi->private_data; struct snd_virmidi *vmidi = substream->runtime->private_data; - snd_midi_event_free(vmidi->parser); + + write_lock_irq(&rdev->filelist_lock); list_del(&vmidi->list); + write_unlock_irq(&rdev->filelist_lock); + snd_midi_event_free(vmidi->parser); substream->runtime->private_data = NULL; kfree(vmidi); return 0; -- cgit v1.2.3 From cc85f7a634cfaf9f0713c6aa06d08817424db37a Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Mon, 1 Feb 2016 12:04:55 +0100 Subject: ALSA: rawmidi: Remove kernel WARNING for NULL user-space buffer check NULL user-space buffer can be passed even in a normal path, thus it's not good to spew a kernel warning with stack trace at each time. Just drop snd_BUG_ON() macro usage there. BugLink: http://lkml.kernel.org/r/CACT4Y+YfVJ3L+q0i-4vyQVyyPD7V=OMX0PWPi29x9Bo3QaBLdw@mail.gmail.com Reported-by: Dmitry Vyukov Cc: Signed-off-by: Takashi Iwai --- sound/core/rawmidi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sound') diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c index a7759846fbaa..f75d1656272c 100644 --- a/sound/core/rawmidi.c +++ b/sound/core/rawmidi.c @@ -1178,7 +1178,7 @@ static long snd_rawmidi_kernel_write1(struct snd_rawmidi_substream *substream, long count1, result; struct snd_rawmidi_runtime *runtime = substream->runtime; - if (snd_BUG_ON(!kernelbuf && !userbuf)) + if (!kernelbuf && !userbuf) return -EINVAL; if (snd_BUG_ON(!runtime->buffer)) return -EINVAL; -- cgit v1.2.3 From b248371628aad599a48540962f6b85a21a8a0c3f Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Sun, 31 Jan 2016 10:32:37 +0100 Subject: ALSA: pcm: Fix potential deadlock in OSS emulation There are potential deadlocks in PCM OSS emulation code while accessing read/write and mmap concurrently. This comes from the infamous mmap_sem usage in copy_from/to_user(). Namely, snd_pcm_oss_write() -> &runtime->oss.params_lock -> copy_to_user() -> &mm->mmap_sem mmap() -> &mm->mmap_sem -> snd_pcm_oss_mmap() -> &runtime->oss.params_lock Since we can't avoid taking params_lock from mmap code path, use trylock variant and aborts with -EAGAIN as a workaround of this AB/BA deadlock. BugLink: http://lkml.kernel.org/r/CACT4Y+bVrBKDG0G2_AcUgUQa+X91VKTeS4v+wN7BSHwHtqn3kQ@mail.gmail.com Reported-by: Dmitry Vyukov Cc: Signed-off-by: Takashi Iwai --- sound/core/oss/pcm_oss.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) (limited to 'sound') diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c index 0e73d03b30e3..ebc9fdfe64df 100644 --- a/sound/core/oss/pcm_oss.c +++ b/sound/core/oss/pcm_oss.c @@ -835,7 +835,8 @@ static int choose_rate(struct snd_pcm_substream *substream, return snd_pcm_hw_param_near(substream, params, SNDRV_PCM_HW_PARAM_RATE, best_rate, NULL); } -static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream) +static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream, + bool trylock) { struct snd_pcm_runtime *runtime = substream->runtime; struct snd_pcm_hw_params *params, *sparams; @@ -849,7 +850,10 @@ static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream) struct snd_mask sformat_mask; struct snd_mask mask; - if (mutex_lock_interruptible(&runtime->oss.params_lock)) + if (trylock) { + if (!(mutex_trylock(&runtime->oss.params_lock))) + return -EAGAIN; + } else if (mutex_lock_interruptible(&runtime->oss.params_lock)) return -EINTR; sw_params = kzalloc(sizeof(*sw_params), GFP_KERNEL); params = kmalloc(sizeof(*params), GFP_KERNEL); @@ -1092,7 +1096,7 @@ static int snd_pcm_oss_get_active_substream(struct snd_pcm_oss_file *pcm_oss_fil if (asubstream == NULL) asubstream = substream; if (substream->runtime->oss.params) { - err = snd_pcm_oss_change_params(substream); + err = snd_pcm_oss_change_params(substream, false); if (err < 0) return err; } @@ -1132,7 +1136,7 @@ static int snd_pcm_oss_make_ready(struct snd_pcm_substream *substream) return 0; runtime = substream->runtime; if (runtime->oss.params) { - err = snd_pcm_oss_change_params(substream); + err = snd_pcm_oss_change_params(substream, false); if (err < 0) return err; } @@ -2163,7 +2167,7 @@ static int snd_pcm_oss_get_space(struct snd_pcm_oss_file *pcm_oss_file, int stre runtime = substream->runtime; if (runtime->oss.params && - (err = snd_pcm_oss_change_params(substream)) < 0) + (err = snd_pcm_oss_change_params(substream, false)) < 0) return err; info.fragsize = runtime->oss.period_bytes; @@ -2804,7 +2808,12 @@ static int snd_pcm_oss_mmap(struct file *file, struct vm_area_struct *area) return -EIO; if (runtime->oss.params) { - if ((err = snd_pcm_oss_change_params(substream)) < 0) + /* use mutex_trylock() for params_lock for avoiding a deadlock + * between mmap_sem and params_lock taken by + * copy_from/to_user() in snd_pcm_oss_write/read() + */ + err = snd_pcm_oss_change_params(substream, true); + if (err < 0) return err; } #ifdef CONFIG_SND_PCM_OSS_PLUGINS -- cgit v1.2.3 From 2cdc7b636d55cbcf42e1e6c8accd85e62d3e9ae8 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Sat, 30 Jan 2016 23:30:25 +0100 Subject: ALSA: seq: Fix yet another races among ALSA timer accesses ALSA sequencer may open/close and control ALSA timer instance dynamically either via sequencer events or direct ioctls. These are done mostly asynchronously, and it may call still some timer action like snd_timer_start() while another is calling snd_timer_close(). Since the instance gets removed by snd_timer_close(), it may lead to a use-after-free. This patch tries to address such a race by protecting each snd_timer_*() call via the existing spinlock and also by avoiding the access to timer during close call. BugLink: http://lkml.kernel.org/r/CACT4Y+Z6RzW5MBr-HUdV-8zwg71WQfKTdPpYGvOeS7v4cyurNQ@mail.gmail.com Reported-by: Dmitry Vyukov Cc: Signed-off-by: Takashi Iwai --- sound/core/seq/seq_timer.c | 87 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 67 insertions(+), 20 deletions(-) (limited to 'sound') diff --git a/sound/core/seq/seq_timer.c b/sound/core/seq/seq_timer.c index 82b220c769c1..293104926098 100644 --- a/sound/core/seq/seq_timer.c +++ b/sound/core/seq/seq_timer.c @@ -90,6 +90,9 @@ void snd_seq_timer_delete(struct snd_seq_timer **tmr) void snd_seq_timer_defaults(struct snd_seq_timer * tmr) { + unsigned long flags; + + spin_lock_irqsave(&tmr->lock, flags); /* setup defaults */ tmr->ppq = 96; /* 96 PPQ */ tmr->tempo = 500000; /* 120 BPM */ @@ -105,21 +108,25 @@ void snd_seq_timer_defaults(struct snd_seq_timer * tmr) tmr->preferred_resolution = seq_default_timer_resolution; tmr->skew = tmr->skew_base = SKEW_BASE; + spin_unlock_irqrestore(&tmr->lock, flags); } -void snd_seq_timer_reset(struct snd_seq_timer * tmr) +static void seq_timer_reset(struct snd_seq_timer *tmr) { - unsigned long flags; - - spin_lock_irqsave(&tmr->lock, flags); - /* reset time & songposition */ tmr->cur_time.tv_sec = 0; tmr->cur_time.tv_nsec = 0; tmr->tick.cur_tick = 0; tmr->tick.fraction = 0; +} + +void snd_seq_timer_reset(struct snd_seq_timer *tmr) +{ + unsigned long flags; + spin_lock_irqsave(&tmr->lock, flags); + seq_timer_reset(tmr); spin_unlock_irqrestore(&tmr->lock, flags); } @@ -138,8 +145,11 @@ static void snd_seq_timer_interrupt(struct snd_timer_instance *timeri, tmr = q->timer; if (tmr == NULL) return; - if (!tmr->running) + spin_lock_irqsave(&tmr->lock, flags); + if (!tmr->running) { + spin_unlock_irqrestore(&tmr->lock, flags); return; + } resolution *= ticks; if (tmr->skew != tmr->skew_base) { @@ -148,8 +158,6 @@ static void snd_seq_timer_interrupt(struct snd_timer_instance *timeri, (((resolution & 0xffff) * tmr->skew) >> 16); } - spin_lock_irqsave(&tmr->lock, flags); - /* update timer */ snd_seq_inc_time_nsec(&tmr->cur_time, resolution); @@ -296,26 +304,30 @@ int snd_seq_timer_open(struct snd_seq_queue *q) t->callback = snd_seq_timer_interrupt; t->callback_data = q; t->flags |= SNDRV_TIMER_IFLG_AUTO; + spin_lock_irq(&tmr->lock); tmr->timeri = t; + spin_unlock_irq(&tmr->lock); return 0; } int snd_seq_timer_close(struct snd_seq_queue *q) { struct snd_seq_timer *tmr; + struct snd_timer_instance *t; tmr = q->timer; if (snd_BUG_ON(!tmr)) return -EINVAL; - if (tmr->timeri) { - snd_timer_stop(tmr->timeri); - snd_timer_close(tmr->timeri); - tmr->timeri = NULL; - } + spin_lock_irq(&tmr->lock); + t = tmr->timeri; + tmr->timeri = NULL; + spin_unlock_irq(&tmr->lock); + if (t) + snd_timer_close(t); return 0; } -int snd_seq_timer_stop(struct snd_seq_timer * tmr) +static int seq_timer_stop(struct snd_seq_timer *tmr) { if (! tmr->timeri) return -EINVAL; @@ -326,6 +338,17 @@ int snd_seq_timer_stop(struct snd_seq_timer * tmr) return 0; } +int snd_seq_timer_stop(struct snd_seq_timer *tmr) +{ + unsigned long flags; + int err; + + spin_lock_irqsave(&tmr->lock, flags); + err = seq_timer_stop(tmr); + spin_unlock_irqrestore(&tmr->lock, flags); + return err; +} + static int initialize_timer(struct snd_seq_timer *tmr) { struct snd_timer *t; @@ -358,13 +381,13 @@ static int initialize_timer(struct snd_seq_timer *tmr) return 0; } -int snd_seq_timer_start(struct snd_seq_timer * tmr) +static int seq_timer_start(struct snd_seq_timer *tmr) { if (! tmr->timeri) return -EINVAL; if (tmr->running) - snd_seq_timer_stop(tmr); - snd_seq_timer_reset(tmr); + seq_timer_stop(tmr); + seq_timer_reset(tmr); if (initialize_timer(tmr) < 0) return -EINVAL; snd_timer_start(tmr->timeri, tmr->ticks); @@ -373,14 +396,25 @@ int snd_seq_timer_start(struct snd_seq_timer * tmr) return 0; } -int snd_seq_timer_continue(struct snd_seq_timer * tmr) +int snd_seq_timer_start(struct snd_seq_timer *tmr) +{ + unsigned long flags; + int err; + + spin_lock_irqsave(&tmr->lock, flags); + err = seq_timer_start(tmr); + spin_unlock_irqrestore(&tmr->lock, flags); + return err; +} + +static int seq_timer_continue(struct snd_seq_timer *tmr) { if (! tmr->timeri) return -EINVAL; if (tmr->running) return -EBUSY; if (! tmr->initialized) { - snd_seq_timer_reset(tmr); + seq_timer_reset(tmr); if (initialize_timer(tmr) < 0) return -EINVAL; } @@ -390,11 +424,24 @@ int snd_seq_timer_continue(struct snd_seq_timer * tmr) return 0; } +int snd_seq_timer_continue(struct snd_seq_timer *tmr) +{ + unsigned long flags; + int err; + + spin_lock_irqsave(&tmr->lock, flags); + err = seq_timer_continue(tmr); + spin_unlock_irqrestore(&tmr->lock, flags); + return err; +} + /* return current 'real' time. use timeofday() to get better granularity. */ snd_seq_real_time_t snd_seq_timer_get_cur_time(struct snd_seq_timer *tmr) { snd_seq_real_time_t cur_time; + unsigned long flags; + spin_lock_irqsave(&tmr->lock, flags); cur_time = tmr->cur_time; if (tmr->running) { struct timeval tm; @@ -410,7 +457,7 @@ snd_seq_real_time_t snd_seq_timer_get_cur_time(struct snd_seq_timer *tmr) } snd_seq_sanity_real_time(&cur_time); } - + spin_unlock_irqrestore(&tmr->lock, flags); return cur_time; } -- cgit v1.2.3 From f784beb75ce82f4136f8a0960d3ee872f7109e09 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Sat, 30 Jan 2016 23:09:08 +0100 Subject: ALSA: timer: Fix link corruption due to double start or stop Although ALSA timer code got hardening for races, it still causes use-after-free error. This is however rather a corrupted linked list, not actually the concurrent accesses. Namely, when timer start is triggered twice, list_add_tail() is called twice, too. This ends up with the link corruption and triggers KASAN error. The simplest fix would be replacing list_add_tail() with list_move_tail(), but fundamentally it's the problem that we don't check the double start/stop correctly. So, the right fix here is to add the proper checks to snd_timer_start() and snd_timer_stop() (and their variants). BugLink: http://lkml.kernel.org/r/CACT4Y+ZyPRoMQjmawbvmCEDrkBD2BQuH7R09=eOkf5ESK8kJAw@mail.gmail.com Reported-by: Dmitry Vyukov Cc: Signed-off-by: Takashi Iwai --- sound/core/timer.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) (limited to 'sound') diff --git a/sound/core/timer.c b/sound/core/timer.c index af1f68f7e315..12db60dd147b 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -451,6 +451,10 @@ static int snd_timer_start_slave(struct snd_timer_instance *timeri) unsigned long flags; spin_lock_irqsave(&slave_active_lock, flags); + if (timeri->flags & SNDRV_TIMER_IFLG_RUNNING) { + spin_unlock_irqrestore(&slave_active_lock, flags); + return -EBUSY; + } timeri->flags |= SNDRV_TIMER_IFLG_RUNNING; if (timeri->master && timeri->timer) { spin_lock(&timeri->timer->lock); @@ -475,7 +479,8 @@ int snd_timer_start(struct snd_timer_instance *timeri, unsigned int ticks) return -EINVAL; if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE) { result = snd_timer_start_slave(timeri); - snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_START); + if (result >= 0) + snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_START); return result; } timer = timeri->timer; @@ -484,11 +489,18 @@ int snd_timer_start(struct snd_timer_instance *timeri, unsigned int ticks) if (timer->card && timer->card->shutdown) return -ENODEV; spin_lock_irqsave(&timer->lock, flags); + if (timeri->flags & (SNDRV_TIMER_IFLG_RUNNING | + SNDRV_TIMER_IFLG_START)) { + result = -EBUSY; + goto unlock; + } timeri->ticks = timeri->cticks = ticks; timeri->pticks = 0; result = snd_timer_start1(timer, timeri, ticks); + unlock: spin_unlock_irqrestore(&timer->lock, flags); - snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_START); + if (result >= 0) + snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_START); return result; } @@ -502,6 +514,10 @@ static int _snd_timer_stop(struct snd_timer_instance *timeri, int event) if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE) { spin_lock_irqsave(&slave_active_lock, flags); + if (!(timeri->flags & SNDRV_TIMER_IFLG_RUNNING)) { + spin_unlock_irqrestore(&slave_active_lock, flags); + return -EBUSY; + } timeri->flags &= ~SNDRV_TIMER_IFLG_RUNNING; list_del_init(&timeri->ack_list); list_del_init(&timeri->active_list); @@ -512,6 +528,11 @@ static int _snd_timer_stop(struct snd_timer_instance *timeri, int event) if (!timer) return -EINVAL; spin_lock_irqsave(&timer->lock, flags); + if (!(timeri->flags & (SNDRV_TIMER_IFLG_RUNNING | + SNDRV_TIMER_IFLG_START))) { + spin_unlock_irqrestore(&timer->lock, flags); + return -EBUSY; + } list_del_init(&timeri->ack_list); list_del_init(&timeri->active_list); if (timer->card && timer->card->shutdown) { @@ -581,10 +602,15 @@ int snd_timer_continue(struct snd_timer_instance *timeri) if (timer->card && timer->card->shutdown) return -ENODEV; spin_lock_irqsave(&timer->lock, flags); + if (timeri->flags & SNDRV_TIMER_IFLG_RUNNING) { + result = -EBUSY; + goto unlock; + } if (!timeri->cticks) timeri->cticks = 1; timeri->pticks = 0; result = snd_timer_start1(timer, timeri, timer->sticks); + unlock: spin_unlock_irqrestore(&timer->lock, flags); snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_CONTINUE); return result; -- cgit v1.2.3 From f146357f069e71aff8e474c625bcebcd3094b3ab Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 2 Feb 2016 14:14:10 +0100 Subject: ALSA: timer: Sync timer deletion at closing the system timer ALSA timer core framework has no sync point at stopping because it's called inside the spinlock. Thus we need a sync point at close for avoiding the stray timer task. This is simply done by implementing the close callback just calling del_timer_sync(). (It's harmless to call it unconditionally, as the core timer itself cares of the already deleted timer instance.) Signed-off-by: Takashi Iwai --- sound/core/timer.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'sound') diff --git a/sound/core/timer.c b/sound/core/timer.c index 12db60dd147b..b419e612f987 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -1058,11 +1058,21 @@ static int snd_timer_s_stop(struct snd_timer * timer) return 0; } +static int snd_timer_s_close(struct snd_timer *timer) +{ + struct snd_timer_system_private *priv; + + priv = (struct snd_timer_system_private *)timer->private_data; + del_timer_sync(&priv->tlist); + return 0; +} + static struct snd_timer_hardware snd_timer_system = { .flags = SNDRV_TIMER_HW_FIRST | SNDRV_TIMER_HW_TASKLET, .resolution = 1000000000L / HZ, .ticks = 10000000L, + .close = snd_timer_s_close, .start = snd_timer_s_start, .stop = snd_timer_s_stop }; -- cgit v1.2.3 From 4231430da9607fb2eb7ea92f3b93ceef3bc2ed93 Mon Sep 17 00:00:00 2001 From: Kailang Yang Date: Wed, 3 Feb 2016 15:03:50 +0800 Subject: ALSA: hda/realtek - New codec support of ALC225 Add new support for ALC225, yet another variant of ALC298 codec. Signed-off-by: Kailang Yang Cc: # 4.4+ Signed-off-by: Takashi Iwai --- sound/pci/hda/patch_realtek.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'sound') diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 33753244f48f..0b05ae2bc2f7 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -327,6 +327,7 @@ static void alc_fill_eapd_coef(struct hda_codec *codec) case 0x10ec0292: alc_update_coef_idx(codec, 0x4, 1<<15, 0); break; + case 0x10ec0225: case 0x10ec0233: case 0x10ec0255: case 0x10ec0256: @@ -900,6 +901,7 @@ static struct alc_codec_rename_pci_table rename_pci_tbl[] = { { 0x10ec0899, 0x1028, 0, "ALC3861" }, { 0x10ec0298, 0x1028, 0, "ALC3266" }, { 0x10ec0256, 0x1028, 0, "ALC3246" }, + { 0x10ec0225, 0x1028, 0, "ALC3253" }, { 0x10ec0670, 0x1025, 0, "ALC669X" }, { 0x10ec0676, 0x1025, 0, "ALC679X" }, { 0x10ec0282, 0x1043, 0, "ALC3229" }, @@ -2651,6 +2653,7 @@ enum { ALC269_TYPE_ALC298, ALC269_TYPE_ALC255, ALC269_TYPE_ALC256, + ALC269_TYPE_ALC225, }; /* @@ -2680,6 +2683,7 @@ static int alc269_parse_auto_config(struct hda_codec *codec) case ALC269_TYPE_ALC298: case ALC269_TYPE_ALC255: case ALC269_TYPE_ALC256: + case ALC269_TYPE_ALC225: ssids = alc269_ssids; break; default: @@ -5906,6 +5910,9 @@ static int patch_alc269(struct hda_codec *codec) spec->gen.mixer_nid = 0; /* ALC256 does not have any loopback mixer path */ alc_update_coef_idx(codec, 0x36, 1 << 13, 1 << 5); /* Switch pcbeep path to Line in path*/ break; + case 0x10ec0225: + spec->codec_variant = ALC269_TYPE_ALC225; + break; } if (snd_hda_codec_read(codec, 0x51, 0, AC_VERB_PARAMETERS, 0) == 0x10ec5505) { @@ -6796,6 +6803,7 @@ static int patch_alc680(struct hda_codec *codec) */ static const struct hda_device_id snd_hda_id_realtek[] = { HDA_CODEC_ENTRY(0x10ec0221, "ALC221", patch_alc269), + HDA_CODEC_ENTRY(0x10ec0225, "ALC225", patch_alc269), HDA_CODEC_ENTRY(0x10ec0231, "ALC231", patch_alc269), HDA_CODEC_ENTRY(0x10ec0233, "ALC233", patch_alc269), HDA_CODEC_ENTRY(0x10ec0235, "ALC233", patch_alc269), -- cgit v1.2.3 From cfc5a845e62853edd36e564c23c64588f4adcae6 Mon Sep 17 00:00:00 2001 From: Kailang Yang Date: Wed, 3 Feb 2016 15:20:39 +0800 Subject: ALSA: hda/realtek - Support Dell headset mode for ALC225 Dell create new platform with ALC298 codec. This patch will enable headset mode for ALC225/ALC3253 platform. Signed-off-by: Kailang Yang Cc: # v4.4+ Signed-off-by: Takashi Iwai --- sound/pci/hda/patch_realtek.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'sound') diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 0b05ae2bc2f7..39866e5055a5 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -5564,6 +5564,9 @@ static const struct hda_model_fixup alc269_fixup_models[] = { {.id = ALC292_FIXUP_TPT440, .name = "tpt440"}, {} }; +#define ALC225_STANDARD_PINS \ + {0x12, 0xb7a60130}, \ + {0x21, 0x04211020} #define ALC256_STANDARD_PINS \ {0x12, 0x90a60140}, \ @@ -5585,6 +5588,12 @@ static const struct hda_model_fixup alc269_fixup_models[] = { {0x21, 0x03211020} static const struct snd_hda_pin_quirk alc269_pin_fixup_tbl[] = { + SND_HDA_PIN_QUIRK(0x10ec0225, 0x1028, "Dell", ALC269_FIXUP_DELL1_MIC_NO_PRESENCE, + ALC225_STANDARD_PINS, + {0x14, 0x901701a0}), + SND_HDA_PIN_QUIRK(0x10ec0225, 0x1028, "Dell", ALC269_FIXUP_DELL1_MIC_NO_PRESENCE, + ALC225_STANDARD_PINS, + {0x14, 0x901701b0}), SND_HDA_PIN_QUIRK(0x10ec0255, 0x1028, "Dell", ALC255_FIXUP_DELL2_MIC_NO_PRESENCE, {0x14, 0x90170110}, {0x21, 0x02211020}), -- cgit v1.2.3 From 4cc9b9d627af2c443cf98e651e3738d84f991cec Mon Sep 17 00:00:00 2001 From: Kailang Yang Date: Wed, 3 Feb 2016 15:09:35 +0800 Subject: ALSA: hda/realtek - Support headset mode for ALC225 Support headset mode for ALC225 platforms. Signed-off-by: Kailang Yang Cc: # v4.4+ Signed-off-by: Takashi Iwai --- sound/pci/hda/patch_realtek.c | 57 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) (limited to 'sound') diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 39866e5055a5..21992fb7035d 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -3662,6 +3662,16 @@ static void alc_headset_mode_unplugged(struct hda_codec *codec) WRITE_COEF(0xb7, 0x802b), {} }; + static struct coef_fw coef0225[] = { + UPDATE_COEF(0x4a, 1<<8, 0), + UPDATE_COEFEX(0x57, 0x05, 1<<14, 0), + UPDATE_COEF(0x63, 3<<14, 3<<14), + UPDATE_COEF(0x4a, 3<<4, 2<<4), + UPDATE_COEF(0x4a, 3<<10, 3<<10), + UPDATE_COEF(0x45, 0x3f<<10, 0x34<<10), + UPDATE_COEF(0x4a, 3<<10, 0), + {} + }; switch (codec->core.vendor_id) { case 0x10ec0255: @@ -3686,6 +3696,9 @@ static void alc_headset_mode_unplugged(struct hda_codec *codec) case 0x10ec0668: alc_process_coef_fw(codec, coef0668); break; + case 0x10ec0225: + alc_process_coef_fw(codec, coef0225); + break; } codec_dbg(codec, "Headset jack set to unplugged mode.\n"); } @@ -3731,6 +3744,13 @@ static void alc_headset_mode_mic_in(struct hda_codec *codec, hda_nid_t hp_pin, UPDATE_COEF(0xc3, 0, 1<<12), {} }; + static struct coef_fw coef0225[] = { + UPDATE_COEFEX(0x57, 0x05, 1<<14, 1<<14), + UPDATE_COEF(0x4a, 3<<4, 2<<4), + UPDATE_COEF(0x63, 3<<14, 0), + {} + }; + switch (codec->core.vendor_id) { case 0x10ec0255: @@ -3776,6 +3796,12 @@ static void alc_headset_mode_mic_in(struct hda_codec *codec, hda_nid_t hp_pin, alc_process_coef_fw(codec, coef0688); snd_hda_set_pin_ctl_cache(codec, mic_pin, PIN_VREF50); break; + case 0x10ec0225: + alc_update_coef_idx(codec, 0x45, 0x3f<<10, 0x31<<10); + snd_hda_set_pin_ctl_cache(codec, hp_pin, 0); + alc_process_coef_fw(codec, coef0225); + snd_hda_set_pin_ctl_cache(codec, mic_pin, PIN_VREF50); + break; } codec_dbg(codec, "Headset jack set to mic-in mode.\n"); } @@ -3888,6 +3914,13 @@ static void alc_headset_mode_ctia(struct hda_codec *codec) WRITE_COEF(0xc3, 0x0000), {} }; + static struct coef_fw coef0225[] = { + UPDATE_COEF(0x45, 0x3f<<10, 0x35<<10), + UPDATE_COEF(0x49, 1<<8, 1<<8), + UPDATE_COEF(0x4a, 7<<6, 7<<6), + UPDATE_COEF(0x4a, 3<<4, 3<<4), + {} + }; switch (codec->core.vendor_id) { case 0x10ec0255: @@ -3916,6 +3949,9 @@ static void alc_headset_mode_ctia(struct hda_codec *codec) case 0x10ec0668: alc_process_coef_fw(codec, coef0688); break; + case 0x10ec0225: + alc_process_coef_fw(codec, coef0225); + break; } codec_dbg(codec, "Headset jack set to iPhone-style headset mode.\n"); } @@ -3959,6 +3995,13 @@ static void alc_headset_mode_omtp(struct hda_codec *codec) WRITE_COEF(0xc3, 0x0000), {} }; + static struct coef_fw coef0225[] = { + UPDATE_COEF(0x45, 0x3f<<10, 0x39<<10), + UPDATE_COEF(0x49, 1<<8, 1<<8), + UPDATE_COEF(0x4a, 7<<6, 7<<6), + UPDATE_COEF(0x4a, 3<<4, 3<<4), + {} + }; switch (codec->core.vendor_id) { case 0x10ec0255: @@ -3987,6 +4030,9 @@ static void alc_headset_mode_omtp(struct hda_codec *codec) case 0x10ec0668: alc_process_coef_fw(codec, coef0688); break; + case 0x10ec0225: + alc_process_coef_fw(codec, coef0225); + break; } codec_dbg(codec, "Headset jack set to Nokia-style headset mode.\n"); } @@ -4018,6 +4064,11 @@ static void alc_determine_headset_type(struct hda_codec *codec) WRITE_COEF(0xc3, 0x0c00), {} }; + static struct coef_fw coef0225[] = { + UPDATE_COEF(0x45, 0x3f<<10, 0x34<<10), + UPDATE_COEF(0x49, 1<<8, 1<<8), + {} + }; switch (codec->core.vendor_id) { case 0x10ec0255: @@ -4062,6 +4113,12 @@ static void alc_determine_headset_type(struct hda_codec *codec) val = alc_read_coef_idx(codec, 0xbe); is_ctia = (val & 0x1c02) == 0x1c02; break; + case 0x10ec0225: + alc_process_coef_fw(codec, coef0225); + msleep(800); + val = alc_read_coef_idx(codec, 0x46); + is_ctia = (val & 0x00f0) == 0x00f0; + break; } codec_dbg(codec, "Headset jack detected iPhone-style headset: %s\n", -- cgit v1.2.3 From 2154cc0e2d4ae15132d005d17e473327c70c9a06 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 3 Feb 2016 12:32:51 +0100 Subject: ALSA: hda - Add fixup for Mac Mini 7,1 model Mac Mini 7,1 model with CS4208 codec reports the headphone jack detection wrongly in an inverted way. Moreover, the advertised pins for the audio input and SPDIF output have actually no jack detection. This patch addresses these issues. The inv_jack_detect flag is set for fixing the headphone jack detection, and the pin configs for audio input and SPDIF output are marked as non-detectable. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=105161 Report-and-tested-by: moosotc@gmail.com Cc: Signed-off-by: Takashi Iwai --- sound/pci/hda/patch_cirrus.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) (limited to 'sound') diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c index a12ae8ac0914..c1c855a6c0af 100644 --- a/sound/pci/hda/patch_cirrus.c +++ b/sound/pci/hda/patch_cirrus.c @@ -614,6 +614,7 @@ enum { CS4208_MAC_AUTO, CS4208_MBA6, CS4208_MBP11, + CS4208_MACMINI, CS4208_GPIO0, }; @@ -621,6 +622,7 @@ static const struct hda_model_fixup cs4208_models[] = { { .id = CS4208_GPIO0, .name = "gpio0" }, { .id = CS4208_MBA6, .name = "mba6" }, { .id = CS4208_MBP11, .name = "mbp11" }, + { .id = CS4208_MACMINI, .name = "macmini" }, {} }; @@ -632,6 +634,7 @@ static const struct snd_pci_quirk cs4208_fixup_tbl[] = { /* codec SSID matching */ static const struct snd_pci_quirk cs4208_mac_fixup_tbl[] = { SND_PCI_QUIRK(0x106b, 0x5e00, "MacBookPro 11,2", CS4208_MBP11), + SND_PCI_QUIRK(0x106b, 0x6c00, "MacMini 7,1", CS4208_MACMINI), SND_PCI_QUIRK(0x106b, 0x7100, "MacBookAir 6,1", CS4208_MBA6), SND_PCI_QUIRK(0x106b, 0x7200, "MacBookAir 6,2", CS4208_MBA6), SND_PCI_QUIRK(0x106b, 0x7b00, "MacBookPro 12,1", CS4208_MBP11), @@ -666,6 +669,24 @@ static void cs4208_fixup_mac(struct hda_codec *codec, snd_hda_apply_fixup(codec, action); } +/* MacMini 7,1 has the inverted jack detection */ +static void cs4208_fixup_macmini(struct hda_codec *codec, + const struct hda_fixup *fix, int action) +{ + static const struct hda_pintbl pincfgs[] = { + { 0x18, 0x00ab9150 }, /* mic (audio-in) jack: disable detect */ + { 0x21, 0x004be140 }, /* SPDIF: disable detect */ + { } + }; + + if (action == HDA_FIXUP_ACT_PRE_PROBE) { + /* HP pin (0x10) has an inverted detection */ + codec->inv_jack_detect = 1; + /* disable the bogus Mic and SPDIF jack detections */ + snd_hda_apply_pincfgs(codec, pincfgs); + } +} + static int cs4208_spdif_sw_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { @@ -709,6 +730,12 @@ static const struct hda_fixup cs4208_fixups[] = { .chained = true, .chain_id = CS4208_GPIO0, }, + [CS4208_MACMINI] = { + .type = HDA_FIXUP_FUNC, + .v.func = cs4208_fixup_macmini, + .chained = true, + .chain_id = CS4208_GPIO0, + }, [CS4208_GPIO0] = { .type = HDA_FIXUP_FUNC, .v.func = cs4208_fixup_gpio0, -- cgit v1.2.3 From 06ab30034ed9c200a570ab13c017bde248ddb2a6 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Sun, 31 Jan 2016 11:57:41 +0100 Subject: ALSA: rawmidi: Make snd_rawmidi_transmit() race-free A kernel WARNING in snd_rawmidi_transmit_ack() is triggered by syzkaller fuzzer: WARNING: CPU: 1 PID: 20739 at sound/core/rawmidi.c:1136 Call Trace: [< inline >] __dump_stack lib/dump_stack.c:15 [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50 [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482 [] warn_slowpath_null+0x29/0x30 kernel/panic.c:515 [] snd_rawmidi_transmit_ack+0x275/0x400 sound/core/rawmidi.c:1136 [] snd_virmidi_output_trigger+0x4b1/0x5a0 sound/core/seq/seq_virmidi.c:163 [< inline >] snd_rawmidi_output_trigger sound/core/rawmidi.c:150 [] snd_rawmidi_kernel_write1+0x549/0x780 sound/core/rawmidi.c:1223 [] snd_rawmidi_write+0x543/0xb30 sound/core/rawmidi.c:1273 [] __vfs_write+0x113/0x480 fs/read_write.c:528 [] vfs_write+0x167/0x4a0 fs/read_write.c:577 [< inline >] SYSC_write fs/read_write.c:624 [] SyS_write+0x111/0x220 fs/read_write.c:616 [] entry_SYSCALL_64_fastpath+0x16/0x7a arch/x86/entry/entry_64.S:185 Also a similar warning is found but in another path: Call Trace: [< inline >] __dump_stack lib/dump_stack.c:15 [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50 [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482 [] warn_slowpath_null+0x29/0x30 kernel/panic.c:515 [] rawmidi_transmit_ack+0x24a/0x3b0 sound/core/rawmidi.c:1133 [] snd_rawmidi_transmit_ack+0x51/0x80 sound/core/rawmidi.c:1163 [] snd_virmidi_output_trigger+0x2b6/0x570 sound/core/seq/seq_virmidi.c:185 [< inline >] snd_rawmidi_output_trigger sound/core/rawmidi.c:150 [] snd_rawmidi_kernel_write1+0x4bb/0x760 sound/core/rawmidi.c:1252 [] snd_rawmidi_write+0x543/0xb30 sound/core/rawmidi.c:1302 [] __vfs_write+0x113/0x480 fs/read_write.c:528 [] vfs_write+0x167/0x4a0 fs/read_write.c:577 [< inline >] SYSC_write fs/read_write.c:624 [] SyS_write+0x111/0x220 fs/read_write.c:616 [] entry_SYSCALL_64_fastpath+0x16/0x7a arch/x86/entry/entry_64.S:185 In the former case, the reason is that virmidi has an open code calling snd_rawmidi_transmit_ack() with the value calculated outside the spinlock. We may use snd_rawmidi_transmit() in a loop just for consuming the input data, but even there, there is a race between snd_rawmidi_transmit_peek() and snd_rawmidi_tranmit_ack(). Similarly in the latter case, it calls snd_rawmidi_transmit_peek() and snd_rawmidi_tranmit_ack() separately without protection, so they are racy as well. The patch tries to address these issues by the following ways: - Introduce the unlocked versions of snd_rawmidi_transmit_peek() and snd_rawmidi_transmit_ack() to be called inside the explicit lock. - Rewrite snd_rawmidi_transmit() to be race-free (the former case). - Make the split calls (the latter case) protected in the rawmidi spin lock. BugLink: http://lkml.kernel.org/r/CACT4Y+YPq1+cYLkadwjWa5XjzF1_Vki1eHnVn-Lm0hzhSpu5PA@mail.gmail.com BugLink: http://lkml.kernel.org/r/CACT4Y+acG4iyphdOZx47Nyq_VHGbpJQK-6xNpiqUjaZYqsXOGw@mail.gmail.com Reported-by: Dmitry Vyukov Tested-by: Dmitry Vyukov Cc: Signed-off-by: Takashi Iwai --- include/sound/rawmidi.h | 4 ++ sound/core/rawmidi.c | 98 ++++++++++++++++++++++++++++++++------------ sound/core/seq/seq_virmidi.c | 17 +++++--- 3 files changed, 88 insertions(+), 31 deletions(-) (limited to 'sound') diff --git a/include/sound/rawmidi.h b/include/sound/rawmidi.h index fdabbb4ddba9..f730b91e472f 100644 --- a/include/sound/rawmidi.h +++ b/include/sound/rawmidi.h @@ -167,6 +167,10 @@ int snd_rawmidi_transmit_peek(struct snd_rawmidi_substream *substream, int snd_rawmidi_transmit_ack(struct snd_rawmidi_substream *substream, int count); int snd_rawmidi_transmit(struct snd_rawmidi_substream *substream, unsigned char *buffer, int count); +int __snd_rawmidi_transmit_peek(struct snd_rawmidi_substream *substream, + unsigned char *buffer, int count); +int __snd_rawmidi_transmit_ack(struct snd_rawmidi_substream *substream, + int count); /* main midi functions */ diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c index f75d1656272c..26ca02248885 100644 --- a/sound/core/rawmidi.c +++ b/sound/core/rawmidi.c @@ -1055,23 +1055,16 @@ int snd_rawmidi_transmit_empty(struct snd_rawmidi_substream *substream) EXPORT_SYMBOL(snd_rawmidi_transmit_empty); /** - * snd_rawmidi_transmit_peek - copy data from the internal buffer + * __snd_rawmidi_transmit_peek - copy data from the internal buffer * @substream: the rawmidi substream * @buffer: the buffer pointer * @count: data size to transfer * - * Copies data from the internal output buffer to the given buffer. - * - * Call this in the interrupt handler when the midi output is ready, - * and call snd_rawmidi_transmit_ack() after the transmission is - * finished. - * - * Return: The size of copied data, or a negative error code on failure. + * This is a variant of snd_rawmidi_transmit_peek() without spinlock. */ -int snd_rawmidi_transmit_peek(struct snd_rawmidi_substream *substream, +int __snd_rawmidi_transmit_peek(struct snd_rawmidi_substream *substream, unsigned char *buffer, int count) { - unsigned long flags; int result, count1; struct snd_rawmidi_runtime *runtime = substream->runtime; @@ -1081,7 +1074,6 @@ int snd_rawmidi_transmit_peek(struct snd_rawmidi_substream *substream, return -EINVAL; } result = 0; - spin_lock_irqsave(&runtime->lock, flags); if (runtime->avail >= runtime->buffer_size) { /* warning: lowlevel layer MUST trigger down the hardware */ goto __skip; @@ -1106,25 +1098,47 @@ int snd_rawmidi_transmit_peek(struct snd_rawmidi_substream *substream, } } __skip: + return result; +} +EXPORT_SYMBOL(__snd_rawmidi_transmit_peek); + +/** + * snd_rawmidi_transmit_peek - copy data from the internal buffer + * @substream: the rawmidi substream + * @buffer: the buffer pointer + * @count: data size to transfer + * + * Copies data from the internal output buffer to the given buffer. + * + * Call this in the interrupt handler when the midi output is ready, + * and call snd_rawmidi_transmit_ack() after the transmission is + * finished. + * + * Return: The size of copied data, or a negative error code on failure. + */ +int snd_rawmidi_transmit_peek(struct snd_rawmidi_substream *substream, + unsigned char *buffer, int count) +{ + struct snd_rawmidi_runtime *runtime = substream->runtime; + int result; + unsigned long flags; + + spin_lock_irqsave(&runtime->lock, flags); + result = __snd_rawmidi_transmit_peek(substream, buffer, count); spin_unlock_irqrestore(&runtime->lock, flags); return result; } EXPORT_SYMBOL(snd_rawmidi_transmit_peek); /** - * snd_rawmidi_transmit_ack - acknowledge the transmission + * __snd_rawmidi_transmit_ack - acknowledge the transmission * @substream: the rawmidi substream * @count: the transferred count * - * Advances the hardware pointer for the internal output buffer with - * the given size and updates the condition. - * Call after the transmission is finished. - * - * Return: The advanced size if successful, or a negative error code on failure. + * This is a variant of __snd_rawmidi_transmit_ack() without spinlock. */ -int snd_rawmidi_transmit_ack(struct snd_rawmidi_substream *substream, int count) +int __snd_rawmidi_transmit_ack(struct snd_rawmidi_substream *substream, int count) { - unsigned long flags; struct snd_rawmidi_runtime *runtime = substream->runtime; if (runtime->buffer == NULL) { @@ -1132,7 +1146,6 @@ int snd_rawmidi_transmit_ack(struct snd_rawmidi_substream *substream, int count) "snd_rawmidi_transmit_ack: output is not active!!!\n"); return -EINVAL; } - spin_lock_irqsave(&runtime->lock, flags); snd_BUG_ON(runtime->avail + count > runtime->buffer_size); runtime->hw_ptr += count; runtime->hw_ptr %= runtime->buffer_size; @@ -1142,9 +1155,32 @@ int snd_rawmidi_transmit_ack(struct snd_rawmidi_substream *substream, int count) if (runtime->drain || snd_rawmidi_ready(substream)) wake_up(&runtime->sleep); } - spin_unlock_irqrestore(&runtime->lock, flags); return count; } +EXPORT_SYMBOL(__snd_rawmidi_transmit_ack); + +/** + * snd_rawmidi_transmit_ack - acknowledge the transmission + * @substream: the rawmidi substream + * @count: the transferred count + * + * Advances the hardware pointer for the internal output buffer with + * the given size and updates the condition. + * Call after the transmission is finished. + * + * Return: The advanced size if successful, or a negative error code on failure. + */ +int snd_rawmidi_transmit_ack(struct snd_rawmidi_substream *substream, int count) +{ + struct snd_rawmidi_runtime *runtime = substream->runtime; + int result; + unsigned long flags; + + spin_lock_irqsave(&runtime->lock, flags); + result = __snd_rawmidi_transmit_ack(substream, count); + spin_unlock_irqrestore(&runtime->lock, flags); + return result; +} EXPORT_SYMBOL(snd_rawmidi_transmit_ack); /** @@ -1160,12 +1196,22 @@ EXPORT_SYMBOL(snd_rawmidi_transmit_ack); int snd_rawmidi_transmit(struct snd_rawmidi_substream *substream, unsigned char *buffer, int count) { + struct snd_rawmidi_runtime *runtime = substream->runtime; + int result; + unsigned long flags; + + spin_lock_irqsave(&runtime->lock, flags); if (!substream->opened) - return -EBADFD; - count = snd_rawmidi_transmit_peek(substream, buffer, count); - if (count < 0) - return count; - return snd_rawmidi_transmit_ack(substream, count); + result = -EBADFD; + else { + count = __snd_rawmidi_transmit_peek(substream, buffer, count); + if (count <= 0) + result = count; + else + result = __snd_rawmidi_transmit_ack(substream, count); + } + spin_unlock_irqrestore(&runtime->lock, flags); + return result; } EXPORT_SYMBOL(snd_rawmidi_transmit); diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c index f71aedfb408c..c82ed3e70506 100644 --- a/sound/core/seq/seq_virmidi.c +++ b/sound/core/seq/seq_virmidi.c @@ -155,21 +155,26 @@ static void snd_virmidi_output_trigger(struct snd_rawmidi_substream *substream, struct snd_virmidi *vmidi = substream->runtime->private_data; int count, res; unsigned char buf[32], *pbuf; + unsigned long flags; if (up) { vmidi->trigger = 1; if (vmidi->seq_mode == SNDRV_VIRMIDI_SEQ_DISPATCH && !(vmidi->rdev->flags & SNDRV_VIRMIDI_SUBSCRIBE)) { - snd_rawmidi_transmit_ack(substream, substream->runtime->buffer_size - substream->runtime->avail); - return; /* ignored */ + while (snd_rawmidi_transmit(substream, buf, + sizeof(buf)) > 0) { + /* ignored */ + } + return; } if (vmidi->event.type != SNDRV_SEQ_EVENT_NONE) { if (snd_seq_kernel_client_dispatch(vmidi->client, &vmidi->event, in_atomic(), 0) < 0) return; vmidi->event.type = SNDRV_SEQ_EVENT_NONE; } + spin_lock_irqsave(&substream->runtime->lock, flags); while (1) { - count = snd_rawmidi_transmit_peek(substream, buf, sizeof(buf)); + count = __snd_rawmidi_transmit_peek(substream, buf, sizeof(buf)); if (count <= 0) break; pbuf = buf; @@ -179,16 +184,18 @@ static void snd_virmidi_output_trigger(struct snd_rawmidi_substream *substream, snd_midi_event_reset_encode(vmidi->parser); continue; } - snd_rawmidi_transmit_ack(substream, res); + __snd_rawmidi_transmit_ack(substream, res); pbuf += res; count -= res; if (vmidi->event.type != SNDRV_SEQ_EVENT_NONE) { if (snd_seq_kernel_client_dispatch(vmidi->client, &vmidi->event, in_atomic(), 0) < 0) - return; + goto out; vmidi->event.type = SNDRV_SEQ_EVENT_NONE; } } } + out: + spin_unlock_irqrestore(&substream->runtime->lock, flags); } else { vmidi->trigger = 0; } -- cgit v1.2.3 From 81f577542af15640cbcb6ef68baa4caa610cbbfc Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 3 Feb 2016 14:41:22 +0100 Subject: ALSA: rawmidi: Fix race at copying & updating the position The rawmidi read and write functions manage runtime stream status such as runtime->appl_ptr and runtime->avail. These point where to copy the new data and how many bytes have been copied (or to be read). The problem is that rawmidi read/write call copy_from_user() or copy_to_user(), and the runtime spinlock is temporarily unlocked and relocked while copying user-space. Since the current code advances and updates the runtime status after the spin unlock/relock, the copy and the update may be asynchronous, and eventually runtime->avail might go to a negative value when many concurrent accesses are done. This may lead to memory corruption in the end. For fixing this race, in this patch, the status update code is performed in the same lock before the temporary unlock. Also, the spinlock is now taken more widely in snd_rawmidi_kernel_read1() for protecting more properly during the whole operation. BugLink: http://lkml.kernel.org/r/CACT4Y+b-dCmNf1GpgPKfDO0ih+uZCL2JV4__j-r1kdhPLSgQCQ@mail.gmail.com Reported-by: Dmitry Vyukov Tested-by: Dmitry Vyukov Cc: Signed-off-by: Takashi Iwai --- sound/core/rawmidi.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) (limited to 'sound') diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c index 26ca02248885..795437b10082 100644 --- a/sound/core/rawmidi.c +++ b/sound/core/rawmidi.c @@ -942,31 +942,36 @@ static long snd_rawmidi_kernel_read1(struct snd_rawmidi_substream *substream, unsigned long flags; long result = 0, count1; struct snd_rawmidi_runtime *runtime = substream->runtime; + unsigned long appl_ptr; + spin_lock_irqsave(&runtime->lock, flags); while (count > 0 && runtime->avail) { count1 = runtime->buffer_size - runtime->appl_ptr; if (count1 > count) count1 = count; - spin_lock_irqsave(&runtime->lock, flags); if (count1 > (int)runtime->avail) count1 = runtime->avail; + + /* update runtime->appl_ptr before unlocking for userbuf */ + appl_ptr = runtime->appl_ptr; + runtime->appl_ptr += count1; + runtime->appl_ptr %= runtime->buffer_size; + runtime->avail -= count1; + if (kernelbuf) - memcpy(kernelbuf + result, runtime->buffer + runtime->appl_ptr, count1); + memcpy(kernelbuf + result, runtime->buffer + appl_ptr, count1); if (userbuf) { spin_unlock_irqrestore(&runtime->lock, flags); if (copy_to_user(userbuf + result, - runtime->buffer + runtime->appl_ptr, count1)) { + runtime->buffer + appl_ptr, count1)) { return result > 0 ? result : -EFAULT; } spin_lock_irqsave(&runtime->lock, flags); } - runtime->appl_ptr += count1; - runtime->appl_ptr %= runtime->buffer_size; - runtime->avail -= count1; - spin_unlock_irqrestore(&runtime->lock, flags); result += count1; count -= count1; } + spin_unlock_irqrestore(&runtime->lock, flags); return result; } @@ -1223,6 +1228,7 @@ static long snd_rawmidi_kernel_write1(struct snd_rawmidi_substream *substream, unsigned long flags; long count1, result; struct snd_rawmidi_runtime *runtime = substream->runtime; + unsigned long appl_ptr; if (!kernelbuf && !userbuf) return -EINVAL; @@ -1243,12 +1249,19 @@ static long snd_rawmidi_kernel_write1(struct snd_rawmidi_substream *substream, count1 = count; if (count1 > (long)runtime->avail) count1 = runtime->avail; + + /* update runtime->appl_ptr before unlocking for userbuf */ + appl_ptr = runtime->appl_ptr; + runtime->appl_ptr += count1; + runtime->appl_ptr %= runtime->buffer_size; + runtime->avail -= count1; + if (kernelbuf) - memcpy(runtime->buffer + runtime->appl_ptr, + memcpy(runtime->buffer + appl_ptr, kernelbuf + result, count1); else if (userbuf) { spin_unlock_irqrestore(&runtime->lock, flags); - if (copy_from_user(runtime->buffer + runtime->appl_ptr, + if (copy_from_user(runtime->buffer + appl_ptr, userbuf + result, count1)) { spin_lock_irqsave(&runtime->lock, flags); result = result > 0 ? result : -EFAULT; @@ -1256,9 +1269,6 @@ static long snd_rawmidi_kernel_write1(struct snd_rawmidi_substream *substream, } spin_lock_irqsave(&runtime->lock, flags); } - runtime->appl_ptr += count1; - runtime->appl_ptr %= runtime->buffer_size; - runtime->avail -= count1; result += count1; count -= count1; } -- cgit v1.2.3 From 7f0973e973cd74aa40747c9d38844560cd184ee8 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 3 Feb 2016 08:32:44 +0100 Subject: ALSA: seq: Fix lockdep warnings due to double mutex locks The port subscription code uses double mutex locks for source and destination ports, and this may become racy once when wrongly set up. It leads to lockdep warning splat, typically triggered by fuzzer like syzkaller, although the actual deadlock hasn't been seen, so far. This patch simplifies the handling by reducing to two single locks, so that no lockdep warning will be trigger any longer. By splitting to two actions, a still-in-progress element shall be added in one list while handling another. For ignoring this element, a new check is added in deliver_to_subscribers(). Along with it, the code to add/remove the subscribers list element was cleaned up and refactored. BugLink: http://lkml.kernel.org/r/CACT4Y+aKQXV7xkBW9hpQbzaDO7LrUvohxWh-UwMxXjDy-yBD=A@mail.gmail.com Reported-by: Dmitry Vyukov Tested-by: Dmitry Vyukov Cc: Signed-off-by: Takashi Iwai --- sound/core/seq/seq_clientmgr.c | 3 + sound/core/seq/seq_ports.c | 233 +++++++++++++++++++++++------------------ 2 files changed, 133 insertions(+), 103 deletions(-) (limited to 'sound') diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 13cfa815732d..58e79e02f217 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -678,6 +678,9 @@ static int deliver_to_subscribers(struct snd_seq_client *client, else down_read(&grp->list_mutex); list_for_each_entry(subs, &grp->list_head, src_list) { + /* both ports ready? */ + if (atomic_read(&subs->ref_count) != 2) + continue; event->dest = subs->info.dest; if (subs->info.flags & SNDRV_SEQ_PORT_SUBS_TIMESTAMP) /* convert time according to flag with subscription */ diff --git a/sound/core/seq/seq_ports.c b/sound/core/seq/seq_ports.c index 55170a20ae72..921fb2bd8fad 100644 --- a/sound/core/seq/seq_ports.c +++ b/sound/core/seq/seq_ports.c @@ -173,10 +173,6 @@ struct snd_seq_client_port *snd_seq_create_port(struct snd_seq_client *client, } /* */ -enum group_type { - SRC_LIST, DEST_LIST -}; - static int subscribe_port(struct snd_seq_client *client, struct snd_seq_client_port *port, struct snd_seq_port_subs_info *grp, @@ -203,6 +199,20 @@ static struct snd_seq_client_port *get_client_port(struct snd_seq_addr *addr, return NULL; } +static void delete_and_unsubscribe_port(struct snd_seq_client *client, + struct snd_seq_client_port *port, + struct snd_seq_subscribers *subs, + bool is_src, bool ack); + +static inline struct snd_seq_subscribers * +get_subscriber(struct list_head *p, bool is_src) +{ + if (is_src) + return list_entry(p, struct snd_seq_subscribers, src_list); + else + return list_entry(p, struct snd_seq_subscribers, dest_list); +} + /* * remove all subscribers on the list * this is called from port_delete, for each src and dest list. @@ -210,7 +220,7 @@ static struct snd_seq_client_port *get_client_port(struct snd_seq_addr *addr, static void clear_subscriber_list(struct snd_seq_client *client, struct snd_seq_client_port *port, struct snd_seq_port_subs_info *grp, - int grptype) + int is_src) { struct list_head *p, *n; @@ -219,15 +229,13 @@ static void clear_subscriber_list(struct snd_seq_client *client, struct snd_seq_client *c; struct snd_seq_client_port *aport; - if (grptype == SRC_LIST) { - subs = list_entry(p, struct snd_seq_subscribers, src_list); + subs = get_subscriber(p, is_src); + if (is_src) aport = get_client_port(&subs->info.dest, &c); - } else { - subs = list_entry(p, struct snd_seq_subscribers, dest_list); + else aport = get_client_port(&subs->info.sender, &c); - } - list_del(p); - unsubscribe_port(client, port, grp, &subs->info, 0); + delete_and_unsubscribe_port(client, port, subs, is_src, false); + if (!aport) { /* looks like the connected port is being deleted. * we decrease the counter, and when both ports are deleted @@ -235,21 +243,14 @@ static void clear_subscriber_list(struct snd_seq_client *client, */ if (atomic_dec_and_test(&subs->ref_count)) kfree(subs); - } else { - /* ok we got the connected port */ - struct snd_seq_port_subs_info *agrp; - agrp = (grptype == SRC_LIST) ? &aport->c_dest : &aport->c_src; - down_write(&agrp->list_mutex); - if (grptype == SRC_LIST) - list_del(&subs->dest_list); - else - list_del(&subs->src_list); - up_write(&agrp->list_mutex); - unsubscribe_port(c, aport, agrp, &subs->info, 1); - kfree(subs); - snd_seq_port_unlock(aport); - snd_seq_client_unlock(c); + continue; } + + /* ok we got the connected port */ + delete_and_unsubscribe_port(c, aport, subs, !is_src, true); + kfree(subs); + snd_seq_port_unlock(aport); + snd_seq_client_unlock(c); } } @@ -262,8 +263,8 @@ static int port_delete(struct snd_seq_client *client, snd_use_lock_sync(&port->use_lock); /* clear subscribers info */ - clear_subscriber_list(client, port, &port->c_src, SRC_LIST); - clear_subscriber_list(client, port, &port->c_dest, DEST_LIST); + clear_subscriber_list(client, port, &port->c_src, true); + clear_subscriber_list(client, port, &port->c_dest, false); if (port->private_free) port->private_free(port->private_data); @@ -479,85 +480,120 @@ static int match_subs_info(struct snd_seq_port_subscribe *r, return 0; } - -/* connect two ports */ -int snd_seq_port_connect(struct snd_seq_client *connector, - struct snd_seq_client *src_client, - struct snd_seq_client_port *src_port, - struct snd_seq_client *dest_client, - struct snd_seq_client_port *dest_port, - struct snd_seq_port_subscribe *info) +static int check_and_subscribe_port(struct snd_seq_client *client, + struct snd_seq_client_port *port, + struct snd_seq_subscribers *subs, + bool is_src, bool exclusive, bool ack) { - struct snd_seq_port_subs_info *src = &src_port->c_src; - struct snd_seq_port_subs_info *dest = &dest_port->c_dest; - struct snd_seq_subscribers *subs, *s; - int err, src_called = 0; - unsigned long flags; - int exclusive; + struct snd_seq_port_subs_info *grp; + struct list_head *p; + struct snd_seq_subscribers *s; + int err; - subs = kzalloc(sizeof(*subs), GFP_KERNEL); - if (! subs) - return -ENOMEM; - - subs->info = *info; - atomic_set(&subs->ref_count, 2); - - down_write(&src->list_mutex); - down_write_nested(&dest->list_mutex, SINGLE_DEPTH_NESTING); - - exclusive = info->flags & SNDRV_SEQ_PORT_SUBS_EXCLUSIVE ? 1 : 0; + grp = is_src ? &port->c_src : &port->c_dest; err = -EBUSY; + down_write(&grp->list_mutex); if (exclusive) { - if (! list_empty(&src->list_head) || ! list_empty(&dest->list_head)) + if (!list_empty(&grp->list_head)) goto __error; } else { - if (src->exclusive || dest->exclusive) + if (grp->exclusive) goto __error; /* check whether already exists */ - list_for_each_entry(s, &src->list_head, src_list) { - if (match_subs_info(info, &s->info)) - goto __error; - } - list_for_each_entry(s, &dest->list_head, dest_list) { - if (match_subs_info(info, &s->info)) + list_for_each(p, &grp->list_head) { + s = get_subscriber(p, is_src); + if (match_subs_info(&subs->info, &s->info)) goto __error; } } - if ((err = subscribe_port(src_client, src_port, src, info, - connector->number != src_client->number)) < 0) - goto __error; - src_called = 1; - - if ((err = subscribe_port(dest_client, dest_port, dest, info, - connector->number != dest_client->number)) < 0) + err = subscribe_port(client, port, grp, &subs->info, ack); + if (err < 0) { + grp->exclusive = 0; goto __error; + } /* add to list */ - write_lock_irqsave(&src->list_lock, flags); - // write_lock(&dest->list_lock); // no other lock yet - list_add_tail(&subs->src_list, &src->list_head); - list_add_tail(&subs->dest_list, &dest->list_head); - // write_unlock(&dest->list_lock); // no other lock yet - write_unlock_irqrestore(&src->list_lock, flags); + write_lock_irq(&grp->list_lock); + if (is_src) + list_add_tail(&subs->src_list, &grp->list_head); + else + list_add_tail(&subs->dest_list, &grp->list_head); + grp->exclusive = exclusive; + atomic_inc(&subs->ref_count); + write_unlock_irq(&grp->list_lock); + err = 0; + + __error: + up_write(&grp->list_mutex); + return err; +} - src->exclusive = dest->exclusive = exclusive; +static void delete_and_unsubscribe_port(struct snd_seq_client *client, + struct snd_seq_client_port *port, + struct snd_seq_subscribers *subs, + bool is_src, bool ack) +{ + struct snd_seq_port_subs_info *grp; + + grp = is_src ? &port->c_src : &port->c_dest; + down_write(&grp->list_mutex); + write_lock_irq(&grp->list_lock); + if (is_src) + list_del(&subs->src_list); + else + list_del(&subs->dest_list); + grp->exclusive = 0; + write_unlock_irq(&grp->list_lock); + up_write(&grp->list_mutex); + + unsubscribe_port(client, port, grp, &subs->info, ack); +} + +/* connect two ports */ +int snd_seq_port_connect(struct snd_seq_client *connector, + struct snd_seq_client *src_client, + struct snd_seq_client_port *src_port, + struct snd_seq_client *dest_client, + struct snd_seq_client_port *dest_port, + struct snd_seq_port_subscribe *info) +{ + struct snd_seq_subscribers *subs; + bool exclusive; + int err; + + subs = kzalloc(sizeof(*subs), GFP_KERNEL); + if (!subs) + return -ENOMEM; + + subs->info = *info; + atomic_set(&subs->ref_count, 0); + INIT_LIST_HEAD(&subs->src_list); + INIT_LIST_HEAD(&subs->dest_list); + + exclusive = !!(info->flags & SNDRV_SEQ_PORT_SUBS_EXCLUSIVE); + + err = check_and_subscribe_port(src_client, src_port, subs, true, + exclusive, + connector->number != src_client->number); + if (err < 0) + goto error; + err = check_and_subscribe_port(dest_client, dest_port, subs, false, + exclusive, + connector->number != dest_client->number); + if (err < 0) + goto error_dest; - up_write(&dest->list_mutex); - up_write(&src->list_mutex); return 0; - __error: - if (src_called) - unsubscribe_port(src_client, src_port, src, info, - connector->number != src_client->number); + error_dest: + delete_and_unsubscribe_port(src_client, src_port, subs, true, + connector->number != src_client->number); + error: kfree(subs); - up_write(&dest->list_mutex); - up_write(&src->list_mutex); return err; } - /* remove the connection */ int snd_seq_port_disconnect(struct snd_seq_client *connector, struct snd_seq_client *src_client, @@ -567,37 +603,28 @@ int snd_seq_port_disconnect(struct snd_seq_client *connector, struct snd_seq_port_subscribe *info) { struct snd_seq_port_subs_info *src = &src_port->c_src; - struct snd_seq_port_subs_info *dest = &dest_port->c_dest; struct snd_seq_subscribers *subs; int err = -ENOENT; - unsigned long flags; down_write(&src->list_mutex); - down_write_nested(&dest->list_mutex, SINGLE_DEPTH_NESTING); - /* look for the connection */ list_for_each_entry(subs, &src->list_head, src_list) { if (match_subs_info(info, &subs->info)) { - write_lock_irqsave(&src->list_lock, flags); - // write_lock(&dest->list_lock); // no lock yet - list_del(&subs->src_list); - list_del(&subs->dest_list); - // write_unlock(&dest->list_lock); - write_unlock_irqrestore(&src->list_lock, flags); - src->exclusive = dest->exclusive = 0; - unsubscribe_port(src_client, src_port, src, info, - connector->number != src_client->number); - unsubscribe_port(dest_client, dest_port, dest, info, - connector->number != dest_client->number); - kfree(subs); + atomic_dec(&subs->ref_count); /* mark as not ready */ err = 0; break; } } - - up_write(&dest->list_mutex); up_write(&src->list_mutex); - return err; + if (err < 0) + return err; + + delete_and_unsubscribe_port(src_client, src_port, subs, true, + connector->number != src_client->number); + delete_and_unsubscribe_port(dest_client, dest_port, subs, false, + connector->number != dest_client->number); + kfree(subs); + return 0; } -- cgit v1.2.3 From 094fd3be87b0f102589e2d5c3fa5d06b7e20496d Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 4 Feb 2016 17:06:13 +0100 Subject: ALSA: timer: Fix leftover link at closing In ALSA timer core, the active timer instance is managed in active_list linked list. Each element is added / removed dynamically at timer start, stop and in timer interrupt. The problem is that snd_timer_interrupt() has a thinko and leaves the element in active_list when it's the last opened element. This eventually leads to list corruption or use-after-free error. This hasn't been revealed because we used to delete the list forcibly in snd_timer_stop() in the past. However, the recent fix avoids the double-stop behavior (in commit [f784beb75ce8: ALSA: timer: Fix link corruption due to double start or stop]), and this leak hits reality. This patch fixes the link management in snd_timer_interrupt(). Now it simply unlinks no matter which stream is. BugLink: http://lkml.kernel.org/r/CACT4Y+Yy2aukHP-EDp8-ziNqNNmb-NTf=jDWXMP7jB8HDa2vng@mail.gmail.com Reported-by: Dmitry Vyukov Cc: Signed-off-by: Takashi Iwai --- sound/core/timer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'sound') diff --git a/sound/core/timer.c b/sound/core/timer.c index b419e612f987..9b513a05765a 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -744,8 +744,8 @@ void snd_timer_interrupt(struct snd_timer * timer, unsigned long ticks_left) ti->cticks = ti->ticks; } else { ti->flags &= ~SNDRV_TIMER_IFLG_RUNNING; - if (--timer->running) - list_del_init(&ti->active_list); + --timer->running; + list_del_init(&ti->active_list); } if ((timer->hw.flags & SNDRV_TIMER_HW_TASKLET) || (ti->flags & SNDRV_TIMER_IFLG_FAST)) -- cgit v1.2.3 From ddce57a6f0a2d8d1bfacfa77f06043bc760403c2 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 2 Feb 2016 15:27:36 +0100 Subject: ALSA: dummy: Implement timer backend switching more safely Currently the selected timer backend is referred at any moment from the running PCM callbacks. When the backend is switched, it's possible to lead to inconsistency from the running backend. This was pointed by syzkaller fuzzer, and the commit [7ee96216c31a: ALSA: dummy: Disable switching timer backend via sysfs] disabled the dynamic switching for avoiding the crash. This patch improves the handling of timer backend switching. It keeps the reference to the selected backend during the whole operation of an opened stream so that it won't be changed by other streams. Together with this change, the hrtimer parameter is reenabled as writable now. NOTE: this patch also turned out to fix the still remaining race. Namely, ops was still replaced dynamically at dummy_pcm_open: static int dummy_pcm_open(struct snd_pcm_substream *substream) { .... dummy->timer_ops = &dummy_systimer_ops; if (hrtimer) dummy->timer_ops = &dummy_hrtimer_ops; Since dummy->timer_ops is common among all streams, and when the replacement happens during accesses of other streams, it may lead to a crash. This was actually triggered by syzkaller fuzzer and KASAN. This patch rewrites the code not to use the ops shared by all streams any longer, too. BugLink: http://lkml.kernel.org/r/CACT4Y+aZ+xisrpuM6cOXbL21DuM0yVxPYXf4cD4Md9uw0C3dBQ@mail.gmail.com Reported-by: Dmitry Vyukov Cc: Signed-off-by: Takashi Iwai --- sound/drivers/dummy.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) (limited to 'sound') diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c index bde33308f0d6..c0f8f613f1f1 100644 --- a/sound/drivers/dummy.c +++ b/sound/drivers/dummy.c @@ -87,7 +87,7 @@ MODULE_PARM_DESC(pcm_substreams, "PCM substreams # (1-128) for dummy driver."); module_param(fake_buffer, bool, 0444); MODULE_PARM_DESC(fake_buffer, "Fake buffer allocations."); #ifdef CONFIG_HIGH_RES_TIMERS -module_param(hrtimer, bool, 0444); +module_param(hrtimer, bool, 0644); MODULE_PARM_DESC(hrtimer, "Use hrtimer as the timer source."); #endif @@ -109,6 +109,9 @@ struct dummy_timer_ops { snd_pcm_uframes_t (*pointer)(struct snd_pcm_substream *); }; +#define get_dummy_ops(substream) \ + (*(const struct dummy_timer_ops **)(substream)->runtime->private_data) + struct dummy_model { const char *name; int (*playback_constraints)(struct snd_pcm_runtime *runtime); @@ -137,7 +140,6 @@ struct snd_dummy { int iobox; struct snd_kcontrol *cd_volume_ctl; struct snd_kcontrol *cd_switch_ctl; - const struct dummy_timer_ops *timer_ops; }; /* @@ -231,6 +233,8 @@ static struct dummy_model *dummy_models[] = { */ struct dummy_systimer_pcm { + /* ops must be the first item */ + const struct dummy_timer_ops *timer_ops; spinlock_t lock; struct timer_list timer; unsigned long base_time; @@ -366,6 +370,8 @@ static const struct dummy_timer_ops dummy_systimer_ops = { */ struct dummy_hrtimer_pcm { + /* ops must be the first item */ + const struct dummy_timer_ops *timer_ops; ktime_t base_time; ktime_t period_time; atomic_t running; @@ -492,31 +498,25 @@ static const struct dummy_timer_ops dummy_hrtimer_ops = { static int dummy_pcm_trigger(struct snd_pcm_substream *substream, int cmd) { - struct snd_dummy *dummy = snd_pcm_substream_chip(substream); - switch (cmd) { case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_RESUME: - return dummy->timer_ops->start(substream); + return get_dummy_ops(substream)->start(substream); case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_SUSPEND: - return dummy->timer_ops->stop(substream); + return get_dummy_ops(substream)->stop(substream); } return -EINVAL; } static int dummy_pcm_prepare(struct snd_pcm_substream *substream) { - struct snd_dummy *dummy = snd_pcm_substream_chip(substream); - - return dummy->timer_ops->prepare(substream); + return get_dummy_ops(substream)->prepare(substream); } static snd_pcm_uframes_t dummy_pcm_pointer(struct snd_pcm_substream *substream) { - struct snd_dummy *dummy = snd_pcm_substream_chip(substream); - - return dummy->timer_ops->pointer(substream); + return get_dummy_ops(substream)->pointer(substream); } static struct snd_pcm_hardware dummy_pcm_hardware = { @@ -562,17 +562,19 @@ static int dummy_pcm_open(struct snd_pcm_substream *substream) struct snd_dummy *dummy = snd_pcm_substream_chip(substream); struct dummy_model *model = dummy->model; struct snd_pcm_runtime *runtime = substream->runtime; + const struct dummy_timer_ops *ops; int err; - dummy->timer_ops = &dummy_systimer_ops; + ops = &dummy_systimer_ops; #ifdef CONFIG_HIGH_RES_TIMERS if (hrtimer) - dummy->timer_ops = &dummy_hrtimer_ops; + ops = &dummy_hrtimer_ops; #endif - err = dummy->timer_ops->create(substream); + err = ops->create(substream); if (err < 0) return err; + get_dummy_ops(substream) = ops; runtime->hw = dummy->pcm_hw; if (substream->pcm->device & 1) { @@ -594,7 +596,7 @@ static int dummy_pcm_open(struct snd_pcm_substream *substream) err = model->capture_constraints(substream->runtime); } if (err < 0) { - dummy->timer_ops->free(substream); + get_dummy_ops(substream)->free(substream); return err; } return 0; @@ -602,8 +604,7 @@ static int dummy_pcm_open(struct snd_pcm_substream *substream) static int dummy_pcm_close(struct snd_pcm_substream *substream) { - struct snd_dummy *dummy = snd_pcm_substream_chip(substream); - dummy->timer_ops->free(substream); + get_dummy_ops(substream)->free(substream); return 0; } -- cgit v1.2.3