Age | Commit message (Collapse) | Author | Files | Lines |
|
We can simplify the code gracefully with new guard() macro and co for
automatic cleanup of locks.
Only the code refactoring, and no functional changes.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/r/20240227085306.9764-22-tiwai@suse.de
|
|
There are common patterns where a temporary buffer is allocated and
freed at the exit, and those can be simplified with the recent cleanup
mechanism via __free(kfree).
A caveat is that some allocations are memdup_user() and they return an
error pointer instead of NULL. Those need special cares and the value
has to be cleared with no_free_ptr() at the allocation error path.
Other than that, the conversions are straightforward.
No functional changes, only code refactoring.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/r/20240222111509.28390-2-tiwai@suse.de
|
|
Fix snd_pcm_format_name so it won't return NULL-pointer in case if it
can't find the format in the 'snd_pcm_format_names' list. Return
"Unknown" instead, as it is done if the number passed to the function
is larger than a list size.
Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
Link: https://lore.kernel.org/r/20240125223522.1122765-2-ivan.orlov0322@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Add 4 missing formats to 'snd_pcm_format_names' array in order to be
able to get their names with 'snd_pcm_format_name' function.
Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
Link: https://lore.kernel.org/r/20240125223522.1122765-1-ivan.orlov0322@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Pull CS35L41 codec extension series.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The pcm state can be SNDRV_PCM_STATE_DISCONNECTED at disconnect
callback, and there is not an entry of SNDRV_PCM_STATE_DISCONNECTED
in snd_pcm_state_names.
This patch adds the missing entry to resolve this issue.
cat /proc/asound/card2/pcm0p/sub0/status
That results in stack traces like the following:
[ 99.702732][ T5171] Unexpected kernel BRK exception at EL1
[ 99.702774][ T5171] Internal error: BRK handler: f2005512 [#1] PREEMPT SMP
[ 99.703858][ T5171] Modules linked in: bcmdhd(E) (...)
[ 99.747425][ T5171] CPU: 3 PID: 5171 Comm: cat Tainted: G C OE 5.10.189-android13-4-00003-g4a17384380d8-ab11086999 #1
[ 99.748447][ T5171] Hardware name: Rockchip RK3588 CVTE V10 Board (DT)
[ 99.749024][ T5171] pstate: 60400005 (nZCv daif +PAN -UAO -TCO BTYPE=--)
[ 99.749616][ T5171] pc : snd_pcm_substream_proc_status_read+0x264/0x2bc
[ 99.750204][ T5171] lr : snd_pcm_substream_proc_status_read+0xa4/0x2bc
[ 99.750778][ T5171] sp : ffffffc0175abae0
[ 99.751132][ T5171] x29: ffffffc0175abb80 x28: ffffffc009a2c498
[ 99.751665][ T5171] x27: 0000000000000001 x26: ffffff810cbae6e8
[ 99.752199][ T5171] x25: 0000000000400cc0 x24: ffffffc0175abc60
[ 99.752729][ T5171] x23: 0000000000000000 x22: ffffff802f558400
[ 99.753263][ T5171] x21: ffffff81d8d8ff00 x20: ffffff81020cdc00
[ 99.753795][ T5171] x19: ffffff802d110000 x18: ffffffc014fbd058
[ 99.754326][ T5171] x17: 0000000000000000 x16: 0000000000000000
[ 99.754861][ T5171] x15: 000000000000c276 x14: ffffffff9a976fda
[ 99.755392][ T5171] x13: 0000000065689089 x12: 000000000000d72e
[ 99.755923][ T5171] x11: ffffff802d110000 x10: 00000000000000e0
[ 99.756457][ T5171] x9 : 9c431600c8385d00 x8 : 0000000000000008
[ 99.756990][ T5171] x7 : 0000000000000000 x6 : 000000000000003f
[ 99.757522][ T5171] x5 : 0000000000000040 x4 : ffffffc0175abb70
[ 99.758056][ T5171] x3 : 0000000000000001 x2 : 0000000000000001
[ 99.758588][ T5171] x1 : 0000000000000000 x0 : 0000000000000000
[ 99.759123][ T5171] Call trace:
[ 99.759404][ T5171] snd_pcm_substream_proc_status_read+0x264/0x2bc
[ 99.759958][ T5171] snd_info_seq_show+0x54/0xa4
[ 99.760370][ T5171] seq_read_iter+0x19c/0x7d4
[ 99.760770][ T5171] seq_read+0xf0/0x128
[ 99.761117][ T5171] proc_reg_read+0x100/0x1f8
[ 99.761515][ T5171] vfs_read+0xf4/0x354
[ 99.761869][ T5171] ksys_read+0x7c/0x148
[ 99.762226][ T5171] __arm64_sys_read+0x20/0x30
[ 99.762625][ T5171] el0_svc_common+0xd0/0x1e4
[ 99.763023][ T5171] el0_svc+0x28/0x98
[ 99.763358][ T5171] el0_sync_handler+0x8c/0xf0
[ 99.763759][ T5171] el0_sync+0x1b8/0x1c0
[ 99.764118][ T5171] Code: d65f03c0 b9406102 17ffffae 94191565 (d42aa240)
[ 99.764715][ T5171] ---[ end trace 1eeffa3e17c58e10 ]---
[ 99.780720][ T5171] Kernel panic - not syncing: BRK handler: Fatal exception
Signed-off-by: Jason Zhang <jason.zhang@rock-chips.com>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20231206013139.20506-1-jason.zhang@rock-chips.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Improve granularity of format selection for S32/U32 formats by adding
constants representing 20, 24 and MAX most significant bits.
The MAX means the maximum number of significant bits which can
the physical format hold. For 32-bit formats, MAX is related
to 32 bits. For 8-bit formats, MAX is related to 8 bits etc.
As there is only one user currently (format S32_LE), subformat is
represented by a simple u32 and stores flags only for that one user
alone. The approach of subformat being part of struct snd_pcm_hardware
is a compromise between ALSA and ASoC allowing for
hw_params-intersection code to be alloc/free-less while not adding any
new responsibilities to ASoC runtime structures.
Acked-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
Co-developed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Link: https://lore.kernel.org/r/20231117120610.1755254-2-cezary.rojewski@intel.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
So far we use the embedded struct device for each PCM substreams in
struct snd_pcm. This may result in UAF when the delayed kobj release
is used; each corresponding struct device is still accessed at the
(delayed) device release, while the snd_pcm object may be already
gone.
As a workaround, detach the struct device from the snd_pcm object by
allocating via the new snd_device_alloc() helper.
A caveat is that we store the PCM substream pointer to drvdata since
the device resume and others require the access to it.
This patch is based on the fix Curtis posted initially. In this
patch, the changes are split and use the new helper function instead.
Link: https://lore.kernel.org/r/20230801171928.1460120-1-cujomalainey@chromium.org
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
Tested-by: Curtis Malainey <cujomalainey@chromium.org>
Link: https://lore.kernel.org/r/20230816160252.23396-4-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
So far, snd_ctl_remove() requires its caller to take
card->controls_rwsem manually before the call for avoiding possible
races. However, many callers don't care and miss the locking.
Basically it's cumbersome and error-prone to enforce it to each
caller. Moreover, card->controls_rwsem is a field that should be used
only by internal or proper helpers, and it's not to be touched at
random external places.
This patch is an attempt to make those calls more consistent: now
snd_ctl_remove() takes the card->controls_rwsem internally, just like
other API functions for kctls. Since a few callers already take the
controls_rwsem locks, the patch removes those locks at the same time,
too.
Link: https://lore.kernel.org/r/20230718141304.1032-5-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
In the PCM core and driver code, there are lots place referring to the
current PCM state via runtime->status->state. This patch introduced a
local PCM state in runtime itself and replaces those references with
runtime->state. It has improvements in two aspects:
- The reduction of a indirect access leads to more code optimization
- It avoids a possible (unexpected) modification of the state via mmap
of the status record
The status->state is updated together with runtime->state, so that
user-space can still read the current state via mmap like before,
too.
This patch touches only the ALSA core code. The changes in each
driver will follow in later patches.
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Link: https://lore.kernel.org/r/20220926135558.26580-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
For sysfs outputs, it's safer to use a new helper, sysfs_emit(),
instead of the raw sprintf() & co. This patch replaces such a
sprintf() call straightforwardly with the new helper.
Link: https://lore.kernel.org/r/20220801165639.26030-6-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
For avoiding the potential deadlock via kill_fasync() call, use the
new fasync helpers to defer the invocation from timer API. Note that
it's merely a workaround.
Reported-by: syzbot+8285e973a41b5aa68902@syzkaller.appspotmail.com
Reported-by: syzbot+669c9abf11a6a011dd09@syzkaller.appspotmail.com
Link: https://lore.kernel.org/r/20220728125945.29533-4-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Each kernel doc comment expects the definition of the return value in
a proper format. This patch adds or fixes the missing entries for PCM
API.
Link: https://lore.kernel.org/r/20220713104759.4365-3-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
syzbot caught a potential deadlock between the PCM
runtime->buffer_mutex and the mm->mmap_lock. It was brought by the
recent fix to cover the racy read/write and other ioctls, and in that
commit, I overlooked a (hopefully only) corner case that may take the
revert lock, namely, the OSS mmap. The OSS mmap operation
exceptionally allows to re-configure the parameters inside the OSS
mmap syscall, where mm->mmap_mutex is already held. Meanwhile, the
copy_from/to_user calls at read/write operations also take the
mm->mmap_lock internally, hence it may lead to a AB/BA deadlock.
A similar problem was already seen in the past and we fixed it with a
refcount (in commit b248371628aa). The former fix covered only the
call paths with OSS read/write and OSS ioctls, while we need to cover
the concurrent access via both ALSA and OSS APIs now.
This patch addresses the problem above by replacing the buffer_mutex
lock in the read/write operations with a refcount similar as we've
used for OSS. The new field, runtime->buffer_accessing, keeps the
number of concurrent read/write operations. Unlike the former
buffer_mutex protection, this protects only around the
copy_from/to_user() calls; the other codes are basically protected by
the PCM stream lock. The refcount can be a negative, meaning blocked
by the ioctls. If a negative value is seen, the read/write aborts
with -EBUSY. In the ioctl side, OTOH, they check this refcount, too,
and set to a negative value for blocking unless it's already being
accessed.
Reported-by: syzbot+6e5c88838328e99c7e1c@syzkaller.appspotmail.com
Fixes: dca947d4d26d ("ALSA: pcm: Fix races among concurrent read/write and buffer changes")
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/000000000000381a0d05db622a81@google.com
Link: https://lore.kernel.org/r/20220330120903.4738-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Currently we have neither proper check nor protection against the
concurrent calls of PCM hw_params and hw_free ioctls, which may result
in a UAF. Since the existing PCM stream lock can't be used for
protecting the whole ioctl operations, we need a new mutex to protect
those racy calls.
This patch introduced a new mutex, runtime->buffer_mutex, and applies
it to both hw_params and hw_free ioctl code paths. Along with it, the
both functions are slightly modified (the mmap_count check is moved
into the state-check block) for code simplicity.
Reported-by: Hu Jiahui <kirin.say@gmail.com>
Cc: <stable@vger.kernel.org>
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Link: https://lore.kernel.org/r/20220322170720.3529-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
snd_ctl_remove() has to be called with card->controls_rwsem held (when
called after the card instantiation). This patch add the missing
rwsem calls around it.
Fixes: a8ff48cb7083 ("ALSA: pcm: Free chmap at PCM free callback, too")
Link: https://lore.kernel.org/r/20211116071314.15065-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Use DEVICE_ATTR_RO() helper instead of plain DEVICE_ATTR(),
which makes the code a bit shorter and easier to read.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Link: https://lore.kernel.org/r/20210524120007.39728-1-yuehaibing@huawei.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
There are a few places doing the same loop iterating all PCM
substreams belonging to the PCM object. Introduce a local helper
macro, for_each_pcm_substream(), to simplify the code.
Link: https://lore.kernel.org/r/20210206203656.15959-5-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The PCM core should perform the sync for the pending stop operations
at disconnection. Otherwise it may lead to unexpected access.
Currently the old user of sync_stop, USB-audio driver, has its own
sync, so this isn't needed, but it's better to guarantee the sync in
the PCM core level.
This patch adds the missing sync_stop call at PCM disconnection
callback. It also assures the IRQ sync if it's specified in the
card. snd_pcm_sync_stop() is slightly modified to be called also for
any PCM substream object now.
Fixes: 1e850beea278 ("ALSA: pcm: Add the support for sync-stop operation")
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210206203656.15959-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
strlcpy is deprecated. see: Documentation/process/deprecated.rst
Change the calls that do not use the strlcpy return value to the
preferred strscpy.
Done with cocci script:
@@
expression e1, e2, e3;
@@
- strlcpy(
+ strscpy(
e1, e2, e3);
This cocci script leaves the instances where the return value is
used unchanged.
After this patch, sound/ has 3 uses of strlcpy() that need to be
manually inspected for conversion and changed one day.
$ git grep -w strlcpy sound/
sound/usb/card.c: len = strlcpy(card->longname, s, sizeof(card->longname));
sound/usb/mixer.c: return strlcpy(buf, p->name, buflen);
sound/usb/mixer.c: return strlcpy(buf, p->names[index], buflen);
Miscellenea:
o Remove trailing whitespace in conversion of sound/core/hwdep.c
Link: https://lore.kernel.org/lkml/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Mark Brown <broonie@kernel.org>
Link: https://lore.kernel.org/r/22b393d1790bb268769d0bab7bacf0866dcb0c14.camel@perches.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
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 <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200902212133.30964-2-pierre-louis.bossart@linux.intel.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
show_pcm_class() returns obviously a short string that can't overflow
PAGE_SIZE. And even if it were to overflow, using snprintf() there is
just wrong, as it doesn't return the correct size.
So simplify with sprintf() instead.
Link: https://lore.kernel.org/r/20200313130223.8908-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Apply const prefix to more possible places: the string tables for PCM
format and co, the table for the PCM type helpers, etc.
Just for minor optimization and no functional changes.
Link: https://lore.kernel.org/r/20200105144823.29547-6-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Now we may declare const for snd_device_ops definitions, so let's do
it for optimization.
There should be no functional changes by this patch.
Link: https://lore.kernel.org/r/20200103081714.9560-5-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The struct snd_pcm_status will use 'timespec' type variables to record
timestamp, which is not year 2038 safe on 32bits system.
Userspace will use SNDRV_PCM_IOCTL_STATUS and SNDRV_PCM_IOCTL_STATUS_EXT
as commands to issue ioctl() to fill the 'snd_pcm_status' structure in
userspace. The command number is always defined through _IOR/_IOW/IORW,
so when userspace changes the definition of 'struct timespec' to use
64-bit types, the command number also changes.
Thus in the kernel, we now need to define two versions of each such ioctl
and corresponding ioctl commands to handle 32bit time_t and 64bit time_t
in native mode:
struct snd_pcm_status32 {
......
s32 trigger_tstamp_sec;
s32 trigger_tstamp_nsec;
......
s32 audio_tstamp_sec;
s32 audio_tstamp_nsec;
......
};
struct snd_pcm_status64 {
......
s32 trigger_tstamp_sec;
s32 trigger_tstamp_nsec;
......
s32 audio_tstamp_sec;
s32 audio_tstamp_nsec;
......
};
Moreover in compat file, we renamed or introduced new structures to handle
32bit/64bit time_t in compatible mode. The 'struct snd_pcm_status32' and
snd_pcm_status_user32() are used to handle 32bit time_t in compat mode.
'struct compat_snd_pcm_status64' and snd_pcm_status_user_compat64() are used
to handle 64bit time_t.
The implicit padding before timespec is made explicit to avoid incompatible
structure layout between 32-bit and 64-bit x86 due to the different
alignment requirements, and the snd_pcm_status structure is now hidden
from the kernel to avoid relying on the timespec definitio definitionn
Finally we can replace SNDRV_PCM_IOCTL_STATUS and SNDRV_PCM_IOCTL_STATUS_EXT
with new commands and introduce new functions to fill new 'struct snd_pcm_status64'
instead of using unsafe 'struct snd_pcm_status'. Then in future, the new
commands can be matched when userspace changes 'timespec' to 64bit type
to make a size change of 'struct snd_pcm_status'. When glibc changes time_t
to 64-bit, any recompiled program will issue ioctl commands that the kernel
does not understand without this patch.
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
|
|
Based on 1 normalized pattern(s):
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation either version 2 of the license or at
your option any later version this program is distributed in the
hope that it will be useful but without any warranty without even
the implied warranty of merchantability or fitness for a particular
purpose see the gnu general public license for more details you
should have received a copy of the gnu general public license along
with this program if not write to the free software foundation inc
59 temple place suite 330 boston ma 02111 1307 usa
extracted by the scancode license scanner the SPDX license identifier
GPL-2.0-or-later
has been chosen to replace the boilerplate/reference in 1334 file(s).
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Allison Randal <allison@lohutok.net>
Reviewed-by: Richard Fontana <rfontana@redhat.com>
Cc: linux-spdx@vger.kernel.org
Link: https://lkml.kernel.org/r/20190527070033.113240726@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
take#2
snd_malloc_pages() and snd_free_pages() are merely thin wrappers of
the standard page allocator / free functions. Even the arguments are
compatible with some standard helpers, so there is little merit of
keeping these wrappers.
This patch replaces the all existing callers of snd_malloc_pages() and
snd_free_pages() with the direct calls of the standard helper
functions. In this version, we use a recently introduced one,
alloc_pages_exact(), which suits better than the old
snd_malloc_pages() implementation for our purposes. Then we can avoid
the waste of pages by alignment to power-of-two.
Since alloc_pages_exact() does split pages, we need no longer
__GFP_COMP flag; or better to say, we must not pass __GFP_COMP to
alloc_pages_exact(). So the former unconditional addition of
__GFP_COMP flag in snd_malloc_pages() is dropped, as well as in most
other places.
Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Acked-by: Michal Hocko <mhocko@suse.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The calls of snd_info_register() are superfluous and should be avoided
at the procfs creation time. They are called at the end of the whole
initialization via snd_card_register(). This patch drops such
superfluous calls, as well as cleaning up the calls of substream proc
entries with a common helper.
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Pull PCM lock refactoring.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The proc files are recursively freed by calling with the root
snd_info_entry object, so we don't have to keep each object for
releasing one by one. Move the release of the PCM stream proc root at
the beginning, so that we can remove the redundant code and resource.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
There are multiple open codes that initialize the same object.
Create a common helper function instead.
Also, use kzalloc() to be safer at creating a group object, and move
the initialization out of the critical section.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Until now we rely on each driver calling snd_pcm_suspend*() explicitly
at its own PM handling. However, this can be done far more easily by
setting the PM ops to each actual snd_pcm device object.
This patch adds the device_type object for PCM stream and assigns to
each PCM stream object. The type contains only the PM ops for system
suspend; we don't need to deal with the resume in general.
The suspend hook simply calls snd_pcm_suspend_all() for the given PCM
streams. This implies that the PM order is correctly put, i.e. PCM is
suspended before the main (or codec) driver, which should be true in
general. If a special ordering is needed, you'd need to adjust the
device PM order manually later.
This patch introduces a new flag, snd_pcm.no_device_suspend, too.
With this flag set, the PCM device object won't invoke
snd_pcm_suspend_all() by itself. This is needed for ASoC who wants to
manage the PM call orders in its serialized way, and the flag is set
in soc_new_pcm() as default.
For the non-ASoC world, we can get rid of the manual snd_pcm_suspend
calls. This will be done in the later patches.
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
stream is indirectly controlled by user-space, hence leading to
a potential exploitation of the Spectre variant 1 vulnerability.
This issue was detected with the help of Smatch:
sound/core/pcm.c:140 snd_pcm_control_ioctl() warn: potential spectre issue 'pcm->streams' [r] (local cap)
Fix this by sanitizing stream before using it to index pcm->streams
Notice that given that speculation windows are large, the policy is
to kill the speculation on the first load and not worry if it can be
completed with a dependent load/store [1].
[1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Cc: stable@vger.kernel.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Basically the xrun injection routine can simply call the standard
helper snd_pcm_stop_xrun(), but with one exception: it may be called
even when the stream is closed.
Make snd_pcm_stop_xrun() more robust and check the NULL runtime state,
and simplify xrun injection code by calling it.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The PCM xrun injection triggers directly snd_pcm_stop() without the
standard xrun handler, hence it's not recorded on the event buffer.
Ditto for snd_pcm_stop_xrun() call and SNDRV_PCM_IOCTL_XRUN ioctl.
They are inconvenient from the debugging POV.
Let's make them to trigger XRUN via the standard helper more
consistently.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Convert the S_<FOO> symbolic permissions to their octal equivalents as
using octal and not symbolic permissions is preferred by many as more
readable.
see: https://lkml.org/lkml/2016/8/2/1945
Done with automated conversion via:
$ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace <files...>
Miscellanea:
o Wrapped one multi-line call to a single line
Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The PCM runtime object is created and freed dynamically at PCM stream
open / close time. This is tracked via substream->runtime, and it's
cleared at snd_pcm_detach_substream().
The runtime object assignment is protected by PCM open_mutex, so for
all PCM operations, it's safely handled. However, each PCM substream
provides also an ALSA timer interface, and user-space can access to
this while closing a PCM substream. This may eventually lead to a
UAF, as snd_pcm_timer_resolution() tries to access the runtime while
clearing it in other side.
Fortunately, it's the only concurrent access from the PCM timer, and
it merely reads runtime->timer_resolution field. So, we can avoid the
race by reordering kfree() and wrapping the substream->runtime
clearance with the corresponding timer lock.
Reported-by: syzbot+8e62ff4e07aa2ce87826@syzkaller.appspotmail.com
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
When the device descriptor is closed, the `substream->runtime` pointer
is freed. But another thread may be in the ioctl handler, case
SNDRV_CTL_IOCTL_PCM_INFO. This case calls snd_pcm_info_user() which
calls snd_pcm_info() which accesses the now freed `substream->runtime`.
Note: this fixes CVE-2017-0861
Signed-off-by: Robb Glasser <rglasser@google.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
So far we assumed that each driver implements the hotplug PCM handling
properly, e.g. dealing with the pending PCM stream at disconnect
callback. But most codes don't care, and it eventually leaves the PCM
stream inconsistent state when an abrupt disconnection like sysfs
unbind happens.
This patch is simple but a big-hammer solution: invoke snd_pcm_stop()
at the common PCM disconnect callback always when the stream is
running.
Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The internal PCM (aka DPCM backend PCM) doesn't need any registration
procedure, thus currently we bail out immediately at dev_register
callback. Similarly, its counterpart, dev_disconnect callback, is
superfluous for the internal PCM. For simplifying and avoiding the
conflicting disconnect call for internal PCM objects, this patch drops
dev_register and dev_disconnect callbacks for the internal ops.
The only uncertain thing by this action is whether skipping the PCM
state change to SNDRV_PCM_STATE_DISCONNECT for the internal PCM is
mandatory. Looking through the current implementations, this doesn't
look so, hence dropping the whole dev_disconnect would make more
sense.
Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The commit c8da9be4a75f ("ALSA: pcm: Adjust nine function calls
together with a variable assignment") contained a badly incorrect
conversion, a "status" PCM procfs creation was replaced with the next
one. Luckily, this could be spotted easily by the kernel runtime
warning.
Fixes: c8da9be4a75f ("ALSA: pcm: Adjust nine function calls together...")
Reported-by: Fabio Estevam <fabio.estevam@nxp.com>
Tested-by: Fabio Estevam <fabio.estevam@nxp.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The script "checkpatch.pl" pointed information out like the following.
ERROR: do not use assignment in if condition
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Add a jump target so that a bit of exception handling can be better reused
at the end of this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by <linux/device.h> work with const
attribute_group. So mark the non-const structs as const.
File size before:
text data bss dec hex filename
9781 240 8 10029 272d sound/core/pcm.o
File size After adding 'const':
text data bss dec hex filename
9813 176 8 9997 270d sound/core/pcm.o
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Several files are used to construct PCM core module, a.k.a snd-pcm.
Although available APIs are described in 'include/sound/pcm.h', some of
them are not exported as symbols in kernel space. Such APIs are just for
module local usage.
This commit adds module local header file and move some function prototypes
into it so that scopes of them are controlled properly and developers
get no confusion from unavailable symbols.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Recent compilers produce a harmless warning for the new pcm_call_notify()
macro when CONFIG_SND_PCM_OSS is disabled:
sound/core/pcm.c: In function 'snd_pcm_free':
sound/core/pcm.c:905:37: error: suggest braces around empty body in an 'if' statement [-Werror=empty-body]
This turns the empty macro into a 'do {} while (0)' statement to avoid
the warning.
Fixes: 58f30d650c7f ("ALSA: pcm: Build pcm notifier code conditionally")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The PCM notifier code is used only by OSS emulation layer, so we can
build it conditionally for reducing the size.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The chmap ctls assigned to PCM streams are freed in the PCM disconnect
callback. However, since the disconnect callback isn't called when
the card gets freed before registering, the chmap ctls may still be
left assigned. They are eventually freed together with other ctls,
but it may cause an Oops at pcm_chmap_ctl_private_free(), as the
function refers to the assigned PCM stream, while the PCM objects have
been already freed beforehand.
The fix is to free the chmap ctls also at PCM free callback, not only
at PCM disconnect.
Reported-by: Laxminath Kasam <b_lkasam@codeaurora.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
because this structure had been removed
This structure was added by 4d96eb255c53 ('ALSA: pcm_lib - add possibility
to log last 10 DMA ring buffer positions') to store PCM pointers
information of latest 10 pointer movements (=XRUN_LOG_CNT). When
CONFIG_SND_PCM_XRUN_DEBUG is configured, 'struct snd_pcm_runtime' has
'hwptr_log' member with a pointer to the structure. When calling
xrun_log() in pcm_lib.c, the structure was allocated to the pointer.
When calling snd_pcm_detach_substream() in pcm.c, the allocated pointer
is released.
In f5914908a5b7 ('ALSA: pcm: Replace PCM hwptr tracking with tracepoints'),
the pointer logging is replaced with using Linux Kernel Tracepoints. The
structure was also removed, while it's just declared. The member and kfree
still remains.
This commit removes the member and related codes. I think this was
overlooked because it brings no errors/warnings to C compilers.
Fixes: f5914908a5b7 ('ALSA: pcm: Replace PCM hwptr tracking with tracepoints')
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The pcm_class sysfs of each PCM substream gives only "none" since the
recent code change to embed the struct device. Fix the code to point
directly to the embedded device object properly.
Fixes: ef46c7af93f9 ('ALSA: pcm: Embed struct device')
Cc: <stable@vger.kernel.org> # v4.0+
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|