From 931522b90813a6f1c9c673ec9a0eb985b39554dc Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 2 Sep 2020 16:21:16 -0500 Subject: ALSA: core: pcm: simplify locking for timers Fix sparse warning: sound/core/pcm.c:999:9: warning: context imbalance in 'snd_pcm_detach_substream' - different lock contexts for basic block There's no real reason to test the same thing twice, and it's simpler have linear sequences. Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20200902212133.30964-2-pierre-louis.bossart@linux.intel.com Signed-off-by: Takashi Iwai --- sound/core/pcm.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'sound/core') diff --git a/sound/core/pcm.c b/sound/core/pcm.c index b6d2331a82f7..be5714f1bb58 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -991,11 +991,13 @@ void snd_pcm_detach_substream(struct snd_pcm_substream *substream) PAGE_ALIGN(sizeof(struct snd_pcm_mmap_control))); kfree(runtime->hw_constraints.rules); /* Avoid concurrent access to runtime via PCM timer interface */ - if (substream->timer) + if (substream->timer) { spin_lock_irq(&substream->timer->lock); - substream->runtime = NULL; - if (substream->timer) + substream->runtime = NULL; spin_unlock_irq(&substream->timer->lock); + } else { + substream->runtime = NULL; + } kfree(runtime); put_pid(substream->pid); substream->pid = NULL; -- cgit v1.2.3 From b1c14124507b86ab554be75ed5fcb2e50bb33c6c Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 2 Sep 2020 16:21:17 -0500 Subject: ALSA: core: memalloc: fix fallthrough position Fix cppcheck, the fallthrough only makes sense within the conditional block sound/core/memalloc.c:161:3: style:inconclusive: Statements following return, break, continue, goto or throw will never be executed. [unreachableCode] fallthrough; ^ Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20200902212133.30964-3-pierre-louis.bossart@linux.intel.com Signed-off-by: Takashi Iwai --- sound/core/memalloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sound/core') diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c index ad74ea9cbff5..0aeeb6244ff6 100644 --- a/sound/core/memalloc.c +++ b/sound/core/memalloc.c @@ -157,8 +157,8 @@ int snd_dma_alloc_pages(int type, struct device *device, size_t size, * so if we fail to malloc, try to fetch memory traditionally. */ dmab->dev.type = SNDRV_DMA_TYPE_DEV; -#endif /* CONFIG_GENERIC_ALLOCATOR */ fallthrough; +#endif /* CONFIG_GENERIC_ALLOCATOR */ case SNDRV_DMA_TYPE_DEV: case SNDRV_DMA_TYPE_DEV_UC: snd_malloc_dev_pages(dmab, size); -- cgit v1.2.3 From b658cbabf8e3e41816f43820f2b03f498e5556d2 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 2 Sep 2020 16:21:18 -0500 Subject: ALSA: core: pcm_memory: dereference pointer after NULL checks Fix cppcheck warnings: sound/core/pcm_memory.c:380:26: warning: Either the condition '!substream' is redundant or there is possible null pointer dereference: substream. [nullPointerRedundantCheck] struct snd_card *card = substream->pcm->card; ^ sound/core/pcm_memory.c:384:6: note: Assuming that condition '!substream' is not redundant if (PCM_RUNTIME_CHECK(substream)) ^ sound/core/pcm_memory.c:380:26: note: Null pointer dereference struct snd_card *card = substream->pcm->card; ^ sound/core/pcm_memory.c:433:26: warning: Either the condition '!substream' is redundant or there is possible null pointer dereference: substream. [nullPointerRedundantCheck] struct snd_card *card = substream->pcm->card; ^ sound/core/pcm_memory.c:436:6: note: Assuming that condition '!substream' is not redundant if (PCM_RUNTIME_CHECK(substream)) ^ sound/core/pcm_memory.c:433:26: note: Null pointer dereference struct snd_card *card = substream->pcm->card; ^ Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20200902212133.30964-4-pierre-louis.bossart@linux.intel.com Signed-off-by: Takashi Iwai --- sound/core/pcm_memory.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'sound/core') diff --git a/sound/core/pcm_memory.c b/sound/core/pcm_memory.c index 1bf6a3d9e0c2..4f03ba8ed0ae 100644 --- a/sound/core/pcm_memory.c +++ b/sound/core/pcm_memory.c @@ -377,7 +377,7 @@ struct page *snd_pcm_sgbuf_ops_page(struct snd_pcm_substream *substream, unsigne */ int snd_pcm_lib_malloc_pages(struct snd_pcm_substream *substream, size_t size) { - struct snd_card *card = substream->pcm->card; + struct snd_card *card; struct snd_pcm_runtime *runtime; struct snd_dma_buffer *dmab = NULL; @@ -387,6 +387,7 @@ int snd_pcm_lib_malloc_pages(struct snd_pcm_substream *substream, size_t size) SNDRV_DMA_TYPE_UNKNOWN)) return -EINVAL; runtime = substream->runtime; + card = substream->pcm->card; if (runtime->dma_buffer_p) { /* perphaps, we might free the large DMA memory region -- cgit v1.2.3 From 63632563c0dfe036215dcba222a0f899d35f7be6 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 2 Sep 2020 16:21:19 -0500 Subject: ALSA: core: timer: remove redundant assignment Cppcheck complains about a possible NULL pointer dereference but it actually looks like the NULL assignment is not needed (same loop is used in other parts of the file without it). Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20200902212133.30964-5-pierre-louis.bossart@linux.intel.com Signed-off-by: Takashi Iwai --- sound/core/timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sound/core') diff --git a/sound/core/timer.c b/sound/core/timer.c index d9f85f2d66a3..1349f5b5be09 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -173,7 +173,7 @@ EXPORT_SYMBOL(snd_timer_instance_free); */ static struct snd_timer *snd_timer_find(struct snd_timer_id *tid) { - struct snd_timer *timer = NULL; + struct snd_timer *timer; list_for_each_entry(timer, &snd_timer_list, device_list) { if (timer->tmr_class != tid->dev_class) -- cgit v1.2.3 From 3bcf8eeb7d979402d3db96fb58bed456a3c66668 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 2 Sep 2020 16:21:20 -0500 Subject: ALSA: core: timer: clarify operator precedence fix cppcheck warning: sound/core/timer.c:1286:9: style: Clarify calculation precedence for '&' and '?'. [clarifyCalculation] ? "running" : "stopped"); ^ Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20200902212133.30964-6-pierre-louis.bossart@linux.intel.com Signed-off-by: Takashi Iwai --- sound/core/timer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'sound/core') diff --git a/sound/core/timer.c b/sound/core/timer.c index 1349f5b5be09..b915d39e7acb 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -1281,8 +1281,8 @@ static void snd_timer_proc_read(struct snd_info_entry *entry, list_for_each_entry(ti, &timer->open_list_head, open_list) snd_iprintf(buffer, " Client %s : %s\n", ti->owner ? ti->owner : "unknown", - ti->flags & (SNDRV_TIMER_IFLG_START | - SNDRV_TIMER_IFLG_RUNNING) + (ti->flags & (SNDRV_TIMER_IFLG_START | + SNDRV_TIMER_IFLG_RUNNING)) ? "running" : "stopped"); } mutex_unlock(®ister_mutex); -- cgit v1.2.3 From cd91fd9f0af5453e66bda9984916ca67686689cf Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 2 Sep 2020 16:21:21 -0500 Subject: ALSA: compress_offload: dereference after checking for NULL pointer Fix cppcheck warning and only dereference once the initial checks are done: sound/core/compress_offload.c:516:38: warning: Either the condition '!stream' is redundant or there is possible null pointer dereference: stream. [nullPointerRedundantCheck] struct snd_compr_runtime *runtime = stream->runtime; ^ sound/core/compress_offload.c:518:17: note: Assuming that condition '!stream' is not redundant if (snd_BUG_ON(!(stream) || !(stream)->runtime)) ^ sound/core/compress_offload.c:516:38: note: Null pointer dereference struct snd_compr_runtime *runtime = stream->runtime; ^ Signed-off-by: Pierre-Louis Bossart Acked-by: Vinod Koul Link: https://lore.kernel.org/r/20200902212133.30964-7-pierre-louis.bossart@linux.intel.com Signed-off-by: Takashi Iwai --- sound/core/compress_offload.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'sound/core') diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c index 0e53f6f31916..e3eb314acb10 100644 --- a/sound/core/compress_offload.c +++ b/sound/core/compress_offload.c @@ -513,10 +513,11 @@ EXPORT_SYMBOL(snd_compr_malloc_pages); int snd_compr_free_pages(struct snd_compr_stream *stream) { - struct snd_compr_runtime *runtime = stream->runtime; + struct snd_compr_runtime *runtime; if (snd_BUG_ON(!(stream) || !(stream)->runtime)) return -EINVAL; + runtime = stream->runtime; if (runtime->dma_area == NULL) return 0; if (runtime->dma_buffer_p != &stream->dma_buffer) { -- cgit v1.2.3 From 9725ce3949a7b3eebf003cb69fa0d7d821179233 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 2 Sep 2020 16:21:22 -0500 Subject: ALSA: compress_offload: remove redundant initialization Fix cppcheck warning: sound/core/compress_offload.c:1044:6: style: Redundant initialization for 'ret'. The initialized value is overwritten before it is read. [redundantInitialization] ret = snd_register_device(SNDRV_DEVICE_TYPE_COMPRESS, ^ sound/core/compress_offload.c:1034:10: note: ret is initialized int ret = -EINVAL; ^ sound/core/compress_offload.c:1044:6: note: ret is overwritten ret = snd_register_device(SNDRV_DEVICE_TYPE_COMPRESS, ^ Signed-off-by: Pierre-Louis Bossart Acked-by: Vinod Koul Link: https://lore.kernel.org/r/20200902212133.30964-8-pierre-louis.bossart@linux.intel.com Signed-off-by: Takashi Iwai --- sound/core/compress_offload.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sound/core') diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c index e3eb314acb10..c1fec932c49d 100644 --- a/sound/core/compress_offload.c +++ b/sound/core/compress_offload.c @@ -1032,7 +1032,7 @@ static const struct file_operations snd_compr_file_ops = { static int snd_compress_dev_register(struct snd_device *device) { - int ret = -EINVAL; + int ret; struct snd_compr *compr; if (snd_BUG_ON(!device || !device->device_data)) -- cgit v1.2.3 From bec206db9db59cbc14993583288e7489e8ab02b1 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 2 Sep 2020 16:21:23 -0500 Subject: ALSA: core: init: use DECLARE_COMPLETION_ONSTACK() macro Follow recommendation in Documentation/scheduler/completion.rst and use macro to declare local 'struct completion' Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20200902212133.30964-9-pierre-louis.bossart@linux.intel.com Signed-off-by: Takashi Iwai --- sound/core/init.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'sound/core') diff --git a/sound/core/init.c b/sound/core/init.c index 0478847ba2b8..764dbe673d48 100644 --- a/sound/core/init.c +++ b/sound/core/init.c @@ -519,10 +519,9 @@ EXPORT_SYMBOL(snd_card_free_when_closed); */ int snd_card_free(struct snd_card *card) { - struct completion released; + DECLARE_COMPLETION_ONSTACK(released); int ret; - init_completion(&released); card->release_completion = &released; ret = snd_card_free_when_closed(card); if (ret) -- cgit v1.2.3 From 93fcef86caafba893f2e13782f4c58b386eab311 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 2 Sep 2020 16:21:30 -0500 Subject: ALSA: rawmidi: (cosmetic) align function parameters fix cppcheck: sound/core/rawmidi.c:1711:49: style:inconclusive: Function 'snd_rawmidi_free' argument 1 names different: declaration 'rawmidi' definition 'rmidi'. [funcArgNamesDifferent] Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20200902212133.30964-16-pierre-louis.bossart@linux.intel.com Signed-off-by: Takashi Iwai --- sound/core/rawmidi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sound/core') diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c index 2a688b711a9a..c78720a3299c 100644 --- a/sound/core/rawmidi.c +++ b/sound/core/rawmidi.c @@ -35,7 +35,7 @@ module_param_array(amidi_map, int, NULL, 0444); MODULE_PARM_DESC(amidi_map, "Raw MIDI device number assigned to 2nd OSS device."); #endif /* CONFIG_SND_OSSEMUL */ -static int snd_rawmidi_free(struct snd_rawmidi *rawmidi); +static int snd_rawmidi_free(struct snd_rawmidi *rmidi); static int snd_rawmidi_dev_free(struct snd_device *device); static int snd_rawmidi_dev_register(struct snd_device *device); static int snd_rawmidi_dev_disconnect(struct snd_device *device); -- cgit v1.2.3 From bf0835957f553aeddec896f3de386562536feee4 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 3 Sep 2020 12:41:22 +0200 Subject: ALSA: timer: Replace tasklet with work The tasklet is an old API that should be deprecated, usually can be converted to another decent API. In ALSA core timer API, the callbacks can be offlined to a tasklet when a flag is set in the timer backend. It can be achieved gracefully with a work queued in the high-prio system workqueue. This patch replaces the usage of tasklet in ALSA timer API with a simple work. Currently the tasklet feature is used only in the system timer and hrtimer backends, so both are patched to use the new flag name SNDRV_TIMER_HW_WORK, too. Link: https://lore.kernel.org/r/20200903104131.21097-3-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/core/hrtimer.c | 2 +- sound/core/timer.c | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) (limited to 'sound/core') diff --git a/sound/core/hrtimer.c b/sound/core/hrtimer.c index c61ba52a530a..e97ff8cccb64 100644 --- a/sound/core/hrtimer.c +++ b/sound/core/hrtimer.c @@ -114,7 +114,7 @@ static int snd_hrtimer_stop(struct snd_timer *t) } static const struct snd_timer_hardware hrtimer_hw __initconst = { - .flags = SNDRV_TIMER_HW_AUTO | SNDRV_TIMER_HW_TASKLET, + .flags = SNDRV_TIMER_HW_AUTO | SNDRV_TIMER_HW_WORK, .open = snd_hrtimer_open, .close = snd_hrtimer_close, .start = snd_hrtimer_start, diff --git a/sound/core/timer.c b/sound/core/timer.c index 227d8152d325..765ea66665a8 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -813,12 +813,12 @@ static void snd_timer_clear_callbacks(struct snd_timer *timer, } /* - * timer tasklet + * timer work * */ -static void snd_timer_tasklet(struct tasklet_struct *t) +static void snd_timer_work(struct work_struct *work) { - struct snd_timer *timer = from_tasklet(timer, t, task_queue); + struct snd_timer *timer = container_of(work, struct snd_timer, task_work); unsigned long flags; if (timer->card && timer->card->shutdown) { @@ -843,7 +843,7 @@ void snd_timer_interrupt(struct snd_timer * timer, unsigned long ticks_left) unsigned long resolution; struct list_head *ack_list_head; unsigned long flags; - int use_tasklet = 0; + bool use_work = false; if (timer == NULL) return; @@ -884,7 +884,7 @@ void snd_timer_interrupt(struct snd_timer * timer, unsigned long ticks_left) --timer->running; list_del_init(&ti->active_list); } - if ((timer->hw.flags & SNDRV_TIMER_HW_TASKLET) || + if ((timer->hw.flags & SNDRV_TIMER_HW_WORK) || (ti->flags & SNDRV_TIMER_IFLG_FAST)) ack_list_head = &timer->ack_list_head; else @@ -919,11 +919,11 @@ void snd_timer_interrupt(struct snd_timer * timer, unsigned long ticks_left) snd_timer_process_callbacks(timer, &timer->ack_list_head); /* do we have any slow callbacks? */ - use_tasklet = !list_empty(&timer->sack_list_head); + use_work = !list_empty(&timer->sack_list_head); spin_unlock_irqrestore(&timer->lock, flags); - if (use_tasklet) - tasklet_schedule(&timer->task_queue); + if (use_work) + queue_work(system_highpri_wq, &timer->task_work); } EXPORT_SYMBOL(snd_timer_interrupt); @@ -967,7 +967,7 @@ int snd_timer_new(struct snd_card *card, char *id, struct snd_timer_id *tid, INIT_LIST_HEAD(&timer->ack_list_head); INIT_LIST_HEAD(&timer->sack_list_head); spin_lock_init(&timer->lock); - tasklet_setup(&timer->task_queue, snd_timer_tasklet); + INIT_WORK(&timer->task_work, snd_timer_work); timer->max_instances = 1000; /* default limit per timer */ if (card != NULL) { timer->module = card->module; @@ -1200,7 +1200,7 @@ static int snd_timer_s_close(struct snd_timer *timer) static const struct snd_timer_hardware snd_timer_system = { - .flags = SNDRV_TIMER_HW_FIRST | SNDRV_TIMER_HW_TASKLET, + .flags = SNDRV_TIMER_HW_FIRST | SNDRV_TIMER_HW_WORK, .resolution = 1000000000L / HZ, .ticks = 10000000L, .close = snd_timer_s_close, -- cgit v1.2.3 From 18d122c0287b29e70bc312a994c7ee79738cec77 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Fri, 18 Sep 2020 11:56:19 +0200 Subject: ALSA: compat_ioctl: avoid compat_alloc_user_space Using compat_alloc_user_space() tends to add complexity to the ioctl handling, so I am trying to remove it everywhere. The two callers in sound/core can rewritten to just call the same code that operates on a kernel pointer as the native handler. Signed-off-by: Arnd Bergmann Link: https://lore.kernel.org/r/20200918095642.1446243-1-arnd@arndb.de Signed-off-by: Takashi Iwai --- sound/core/control.c | 38 +++++++++++++++++++++++++------------- sound/core/control_compat.c | 14 ++++++-------- sound/core/hwdep.c | 27 +++++++++++++++++---------- sound/core/hwdep_compat.c | 23 +++++++---------------- 4 files changed, 55 insertions(+), 47 deletions(-) (limited to 'sound/core') diff --git a/sound/core/control.c b/sound/core/control.c index aa0c0cf182af..e014598142df 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -717,22 +717,19 @@ static int snd_ctl_card_info(struct snd_card *card, struct snd_ctl_file * ctl, } static int snd_ctl_elem_list(struct snd_card *card, - struct snd_ctl_elem_list __user *_list) + struct snd_ctl_elem_list *list) { - struct snd_ctl_elem_list list; struct snd_kcontrol *kctl; struct snd_ctl_elem_id id; unsigned int offset, space, jidx; int err = 0; - if (copy_from_user(&list, _list, sizeof(list))) - return -EFAULT; - offset = list.offset; - space = list.space; + offset = list->offset; + space = list->space; down_read(&card->controls_rwsem); - list.count = card->controls_count; - list.used = 0; + list->count = card->controls_count; + list->used = 0; if (space > 0) { list_for_each_entry(kctl, &card->controls, list) { if (offset >= kctl->count) { @@ -741,12 +738,12 @@ static int snd_ctl_elem_list(struct snd_card *card, } for (jidx = offset; jidx < kctl->count; jidx++) { snd_ctl_build_ioff(&id, kctl, jidx); - if (copy_to_user(list.pids + list.used, &id, + if (copy_to_user(list->pids + list->used, &id, sizeof(id))) { err = -EFAULT; goto out; } - list.used++; + list->used++; if (!--space) goto out; } @@ -755,11 +752,26 @@ static int snd_ctl_elem_list(struct snd_card *card, } out: up_read(&card->controls_rwsem); - if (!err && copy_to_user(_list, &list, sizeof(list))) - err = -EFAULT; return err; } +static int snd_ctl_elem_list_user(struct snd_card *card, + struct snd_ctl_elem_list __user *_list) +{ + struct snd_ctl_elem_list list; + int err; + + if (copy_from_user(&list, _list, sizeof(list))) + return -EFAULT; + err = snd_ctl_elem_list(card, &list); + if (err) + return err; + if (copy_to_user(_list, &list, sizeof(list))) + return -EFAULT; + + return 0; +} + /* Check whether the given kctl info is valid */ static int snd_ctl_check_elem_info(struct snd_card *card, const struct snd_ctl_elem_info *info) @@ -1703,7 +1715,7 @@ static long snd_ctl_ioctl(struct file *file, unsigned int cmd, unsigned long arg case SNDRV_CTL_IOCTL_CARD_INFO: return snd_ctl_card_info(card, ctl, cmd, argp); case SNDRV_CTL_IOCTL_ELEM_LIST: - return snd_ctl_elem_list(card, argp); + return snd_ctl_elem_list_user(card, argp); case SNDRV_CTL_IOCTL_ELEM_INFO: return snd_ctl_elem_info_user(ctl, argp); case SNDRV_CTL_IOCTL_ELEM_READ: diff --git a/sound/core/control_compat.c b/sound/core/control_compat.c index 02df1d7db9a1..1d708aab9c98 100644 --- a/sound/core/control_compat.c +++ b/sound/core/control_compat.c @@ -22,24 +22,22 @@ struct snd_ctl_elem_list32 { static int snd_ctl_elem_list_compat(struct snd_card *card, struct snd_ctl_elem_list32 __user *data32) { - struct snd_ctl_elem_list __user *data; + struct snd_ctl_elem_list data = {}; compat_caddr_t ptr; int err; - data = compat_alloc_user_space(sizeof(*data)); - /* offset, space, used, count */ - if (copy_in_user(data, data32, 4 * sizeof(u32))) + if (copy_from_user(&data, data32, 4 * sizeof(u32))) return -EFAULT; /* pids */ - if (get_user(ptr, &data32->pids) || - put_user(compat_ptr(ptr), &data->pids)) + if (get_user(ptr, &data32->pids)) return -EFAULT; - err = snd_ctl_elem_list(card, data); + data.pids = compat_ptr(ptr); + err = snd_ctl_elem_list(card, &data); if (err < 0) return err; /* copy the result */ - if (copy_in_user(data32, data, 4 * sizeof(u32))) + if (copy_to_user(data32, &data, 4 * sizeof(u32))) return -EFAULT; return 0; } diff --git a/sound/core/hwdep.c b/sound/core/hwdep.c index 21edb8ac95eb..0c029892880a 100644 --- a/sound/core/hwdep.c +++ b/sound/core/hwdep.c @@ -203,28 +203,35 @@ static int snd_hwdep_dsp_status(struct snd_hwdep *hw, } static int snd_hwdep_dsp_load(struct snd_hwdep *hw, - struct snd_hwdep_dsp_image __user *_info) + struct snd_hwdep_dsp_image *info) { - struct snd_hwdep_dsp_image info; int err; if (! hw->ops.dsp_load) return -ENXIO; - memset(&info, 0, sizeof(info)); - if (copy_from_user(&info, _info, sizeof(info))) - return -EFAULT; - if (info.index >= 32) + if (info->index >= 32) return -EINVAL; /* check whether the dsp was already loaded */ - if (hw->dsp_loaded & (1u << info.index)) + if (hw->dsp_loaded & (1u << info->index)) return -EBUSY; - err = hw->ops.dsp_load(hw, &info); + err = hw->ops.dsp_load(hw, info); if (err < 0) return err; - hw->dsp_loaded |= (1u << info.index); + hw->dsp_loaded |= (1u << info->index); return 0; } +static int snd_hwdep_dsp_load_user(struct snd_hwdep *hw, + struct snd_hwdep_dsp_image __user *_info) +{ + struct snd_hwdep_dsp_image info = {}; + + if (copy_from_user(&info, _info, sizeof(info))) + return -EFAULT; + return snd_hwdep_dsp_load(hw, &info); +} + + static long snd_hwdep_ioctl(struct file * file, unsigned int cmd, unsigned long arg) { @@ -238,7 +245,7 @@ static long snd_hwdep_ioctl(struct file * file, unsigned int cmd, case SNDRV_HWDEP_IOCTL_DSP_STATUS: return snd_hwdep_dsp_status(hw, argp); case SNDRV_HWDEP_IOCTL_DSP_LOAD: - return snd_hwdep_dsp_load(hw, argp); + return snd_hwdep_dsp_load_user(hw, argp); } if (hw->ops.ioctl) return hw->ops.ioctl(hw, file, cmd, arg); diff --git a/sound/core/hwdep_compat.c b/sound/core/hwdep_compat.c index bc81db9cb3d4..a0b76706c083 100644 --- a/sound/core/hwdep_compat.c +++ b/sound/core/hwdep_compat.c @@ -19,26 +19,17 @@ struct snd_hwdep_dsp_image32 { static int snd_hwdep_dsp_load_compat(struct snd_hwdep *hw, struct snd_hwdep_dsp_image32 __user *src) { - struct snd_hwdep_dsp_image __user *dst; + struct snd_hwdep_dsp_image info = {}; compat_caddr_t ptr; - u32 val; - dst = compat_alloc_user_space(sizeof(*dst)); - - /* index and name */ - if (copy_in_user(dst, src, 4 + 64)) - return -EFAULT; - if (get_user(ptr, &src->image) || - put_user(compat_ptr(ptr), &dst->image)) - return -EFAULT; - if (get_user(val, &src->length) || - put_user(val, &dst->length)) - return -EFAULT; - if (get_user(val, &src->driver_data) || - put_user(val, &dst->driver_data)) + if (copy_from_user(&info, src, 4 + 64) || + get_user(ptr, &src->image) || + get_user(info.length, &src->length) || + get_user(info.driver_data, &src->driver_data)) return -EFAULT; + info.image = compat_ptr(ptr); - return snd_hwdep_dsp_load(hw, dst); + return snd_hwdep_dsp_load(hw, &info); } enum { -- cgit v1.2.3 From 6564d0ad67efb2d977e130e7448505ee538af016 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 22 Sep 2020 10:49:53 +0200 Subject: ALSA: ctl: Workaround for lockdep warning wrt card->ctl_files_rwlock The recent change in lockdep for read lock caused the deadlock warnings in ALSA control code which uses the read_lock() for notification and else while write_lock_irqsave() is used for adding and removing the list entry. Although a deadlock would practically never hit in a real usage (the addition and the deletion can't happen with the notification), it's better to fix the read_lock() usage in a semantically correct way. This patch replaces the read_lock() calls with read_lock_irqsave() version for avoiding a reported deadlock. The notification code path takes the irq disablement in anyway, and other code paths are very short execution, hence there shouldn't be any big performance hit by this change. Fixes: e918188611f0 ("locking: More accurate annotations for read_lock()") Reported-by: syzbot+561a74f84100162990b2@syzkaller.appspotmail.com Link: https://lore.kernel.org/r/20200922084953.29018-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/core/control.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'sound/core') diff --git a/sound/core/control.c b/sound/core/control.c index e014598142df..421ddc76f264 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -150,14 +150,14 @@ void snd_ctl_notify(struct snd_card *card, unsigned int mask, return; if (card->shutdown) return; - read_lock(&card->ctl_files_rwlock); + read_lock_irqsave(&card->ctl_files_rwlock, flags); #if IS_ENABLED(CONFIG_SND_MIXER_OSS) card->mixer_oss_change_count++; #endif list_for_each_entry(ctl, &card->ctl_files, list) { if (!ctl->subscribed) continue; - spin_lock_irqsave(&ctl->read_lock, flags); + spin_lock(&ctl->read_lock); list_for_each_entry(ev, &ctl->events, list) { if (ev->id.numid == id->numid) { ev->mask |= mask; @@ -174,10 +174,10 @@ void snd_ctl_notify(struct snd_card *card, unsigned int mask, } _found: wake_up(&ctl->change_sleep); - spin_unlock_irqrestore(&ctl->read_lock, flags); + spin_unlock(&ctl->read_lock); kill_fasync(&ctl->fasync, SIGIO, POLL_IN); } - read_unlock(&card->ctl_files_rwlock); + read_unlock_irqrestore(&card->ctl_files_rwlock, flags); } EXPORT_SYMBOL(snd_ctl_notify); @@ -1951,8 +1951,9 @@ int snd_ctl_get_preferred_subdevice(struct snd_card *card, int type) { struct snd_ctl_file *kctl; int subdevice = -1; + unsigned long flags; - read_lock(&card->ctl_files_rwlock); + read_lock_irqsave(&card->ctl_files_rwlock, flags); list_for_each_entry(kctl, &card->ctl_files, list) { if (kctl->pid == task_pid(current)) { subdevice = kctl->preferred_subdevice[type]; @@ -1960,7 +1961,7 @@ int snd_ctl_get_preferred_subdevice(struct snd_card *card, int type) break; } } - read_unlock(&card->ctl_files_rwlock); + read_unlock_irqrestore(&card->ctl_files_rwlock, flags); return subdevice; } EXPORT_SYMBOL_GPL(snd_ctl_get_preferred_subdevice); @@ -2009,13 +2010,14 @@ static int snd_ctl_dev_disconnect(struct snd_device *device) { struct snd_card *card = device->device_data; struct snd_ctl_file *ctl; + unsigned long flags; - read_lock(&card->ctl_files_rwlock); + read_lock_irqsave(&card->ctl_files_rwlock, flags); list_for_each_entry(ctl, &card->ctl_files, list) { wake_up(&ctl->change_sleep); kill_fasync(&ctl->fasync, SIGIO, POLL_ERR); } - read_unlock(&card->ctl_files_rwlock); + read_unlock_irqrestore(&card->ctl_files_rwlock, flags); return snd_unregister_device(&card->ctl_dev); } -- cgit v1.2.3 From 2759caad2600d503c3b0ed800e7e03d2cd7a4c05 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 22 Sep 2020 10:38:56 +0200 Subject: ALSA: seq: oss: Avoid mutex lock for a long-time ioctl Recently we applied a fix to cover the whole OSS sequencer ioctls with the mutex for dealing with the possible races. This works fine in general, but in theory, this may lead to unexpectedly long stall if an ioctl like SNDCTL_SEQ_SYNC is issued and an event with the far future timestamp was queued. For fixing such a potential stall, this patch changes the mutex lock applied conditionally excluding such an ioctl command. Also, change the mutex_lock() with the interruptible version for user to allow escaping from the big-hammer mutex. Fixes: 80982c7e834e ("ALSA: seq: oss: Serialize ioctls") Suggested-by: Pavel Machek Link: https://lore.kernel.org/r/20200922083856.28572-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/core/seq/oss/seq_oss.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'sound/core') diff --git a/sound/core/seq/oss/seq_oss.c b/sound/core/seq/oss/seq_oss.c index c8b9c0b315d8..250a92b18726 100644 --- a/sound/core/seq/oss/seq_oss.c +++ b/sound/core/seq/oss/seq_oss.c @@ -174,9 +174,12 @@ odev_ioctl(struct file *file, unsigned int cmd, unsigned long arg) if (snd_BUG_ON(!dp)) return -ENXIO; - mutex_lock(®ister_mutex); + if (cmd != SNDCTL_SEQ_SYNC && + mutex_lock_interruptible(®ister_mutex)) + return -ERESTARTSYS; rc = snd_seq_oss_ioctl(dp, cmd, arg); - mutex_unlock(®ister_mutex); + if (cmd != SNDCTL_SEQ_SYNC) + mutex_unlock(®ister_mutex); return rc; } -- cgit v1.2.3 From f7b6603c666798a1f8379e692d11d500885f32d8 Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Fri, 23 Oct 2020 18:33:35 +0200 Subject: ALSA: fix kernel-doc markups Kernel-doc markups should use this format: identifier - description There is a common comment marked, instead, with kernel-doc notation. Some identifiers have different names between their prototypes and the kernel-doc markup. Signed-off-by: Mauro Carvalho Chehab Acked-by: Mark Brown Link: https://lore.kernel.org/r/535182d6f55d7a7de293dda9676df68f5f60afc6.1603469755.git.mchehab+huawei@kernel.org Signed-off-by: Takashi Iwai --- sound/core/control.c | 4 ++-- sound/core/pcm_dmaengine.c | 3 ++- sound/core/pcm_lib.c | 2 +- sound/core/pcm_native.c | 4 ++-- 4 files changed, 7 insertions(+), 6 deletions(-) (limited to 'sound/core') diff --git a/sound/core/control.c b/sound/core/control.c index 421ddc76f264..4373de42a5a0 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -1925,8 +1925,8 @@ EXPORT_SYMBOL(snd_ctl_unregister_ioctl); #ifdef CONFIG_COMPAT /** - * snd_ctl_unregister_ioctl - de-register the device-specific compat 32bit - * control-ioctls + * snd_ctl_unregister_ioctl_compat - de-register the device-specific compat + * 32bit control-ioctls * @fcn: ioctl callback function to unregister */ int snd_ctl_unregister_ioctl_compat(snd_kctl_ioctl_func_t fcn) diff --git a/sound/core/pcm_dmaengine.c b/sound/core/pcm_dmaengine.c index 4d059ff2b2e4..4d0e8fe535a1 100644 --- a/sound/core/pcm_dmaengine.c +++ b/sound/core/pcm_dmaengine.c @@ -356,7 +356,8 @@ int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream) EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_close); /** - * snd_dmaengine_pcm_release_chan_close - Close a dmaengine based PCM substream and release channel + * snd_dmaengine_pcm_close_release_chan - Close a dmaengine based PCM + * substream and release channel * @substream: PCM substream * * Releases the DMA channel associated with the PCM substream. diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index d531e1bc2b81..bda3514c7b2d 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -490,7 +490,7 @@ void snd_pcm_set_ops(struct snd_pcm *pcm, int direction, EXPORT_SYMBOL(snd_pcm_set_ops); /** - * snd_pcm_sync - set the PCM sync id + * snd_pcm_set_sync - set the PCM sync id * @substream: the pcm substream * * Sets the PCM sync identifier for the card. diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 9e0b2d73faf6..47b155a49226 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -112,7 +112,7 @@ void snd_pcm_stream_lock(struct snd_pcm_substream *substream) EXPORT_SYMBOL_GPL(snd_pcm_stream_lock); /** - * snd_pcm_stream_lock - Unlock the PCM stream + * snd_pcm_stream_unlock - Unlock the PCM stream * @substream: PCM substream * * This unlocks the PCM stream that has been locked via snd_pcm_stream_lock(). @@ -595,7 +595,7 @@ static void snd_pcm_sync_stop(struct snd_pcm_substream *substream) } /** - * snd_pcm_hw_param_choose - choose a configuration defined by @params + * snd_pcm_hw_params_choose - choose a configuration defined by @params * @pcm: PCM instance * @params: the hw_params instance * -- cgit v1.2.3