| Age | Commit message (Collapse) | Author |
|---|
|
commit e1a7bfe3807974e66f971f2589d4e0197ec0fced upstream.
The procedure for adding a user control element has some window opened
for race against the concurrent removal of a user element. This was
caught by syzkaller, hitting a KASAN use-after-free error.
This patch addresses the bug by wrapping the whole procedure to add a
user control element with the card->controls_rwsem, instead of only
around the increment of card->user_ctl_count.
This required a slight code refactoring, too. The function
snd_ctl_add() is split to two parts: a core function to add the
control element and a part calling it. The former is called from the
function for adding a user control element inside the controls_rwsem.
One change to be noted is that snd_ctl_notify() for adding a control
element gets called inside the controls_rwsem as well while it was
called outside the rwsem. But this should be OK, as snd_ctl_notify()
takes another (finer) rwlock instead of rwsem, and the call of
snd_ctl_notify() inside rwsem is already done in another code path.
Reported-by: syzbot+dc09047bce3820621ba2@syzkaller.appspotmail.com
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
commit 88c54cdf61f508ebcf8da2d819f5dfc03e954d1d upstream.
When user tries to replace the user-defined control TLV, the kernel
checks the change of its content via memcmp(). The problem is that
the kernel passes the return value from memcmp() as is. memcmp()
gives a non-zero negative value depending on the comparison result,
and this shall be recognized as an error code.
The patch covers that corner-case, return 1 properly for the changed
TLV.
Fixes: 8aa9b586e420 ("[ALSA] Control API - more robust TLV implementation")
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Merged 4.8 changes.
|
|
snd_ctl_remove() has a notification for the removal event. It's
superfluous when done during the device got disconnected. Although
the notification itself is mostly harmless, it may potentially be
harmful, and should be suppressed. Actually some components PCM may
free ctl elements during the disconnect or free callbacks, thus it's
no theoretical issue.
This patch adds the check of card->shutdown flag for avoiding
unnecessary notifications after (or during) the disconnect.
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The 'dimen' field in struct snd_ctl_elem_info is used to compose all of
members in the element as multi-dimensional matrix. The field has four
members. Each member represents the width in each dimension level by
element member unit. For example, if the members consist of typical
two dimensional matrix, the dimen[0] represents the number of rows
and dimen[1] represents the number of columns (or vise-versa).
The total members in the matrix should be exactly the same as the number
of members in the element, while current implementation has no validator
of this information. In a view of userspace applications, the information
must be valid so that it cannot cause any bugs such as buffer-over-run.
This commit adds a validator of dimension information for userspace
applications which add new element sets. When they add the element sets
with wrong dimension information, they receive -EINVAL.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
When a TLV ioctl with numid zero is handled, the driver may spew a
kernel warning with a stack trace at each call. The check was
intended obviously only for a kernel driver, but not for a user
interaction. Let's fix it.
This was spotted by syzkaller fuzzer.
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
userspace element
An element instance can have several elements with the same feature.
Some userspace applications can add such an element instance by add
operation with the number of elements. Then, the element instance
gets a memory object to keep states of these elements.
But the element instance has just one memory object for the elements.
This causes the same result to each read/write operations to the
different elements.
This commit fixes this bug by allocating enough memory objects to the
element instance for each of elements.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The commit [39d118677baa: ALSA: ctl: evaluate macro instead of
numerical value] replaced the numbers with constants, but one place
was replaced wrongly with a different type. Fixed now.
Fixes: 39d118677baa ('ALSA: ctl: evaluate macro instead of numerical value')
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
elements
currently some members related identical information are not fiiled
in returned parameter of SNDRV_CTL_IOCTL_ELEM_ADD. This is not better
for userspace application.
This commit copies information to returned value. When failing to copy
into userspace, the added elements are going to be removed. Then, no
applications can lock these elements between adding and removing because
these are already locked.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
for userspace controls
In operations of SNDRV_CTL_IOCTL_ELEM_INFO, identical information in
returned value is cleared. This is not better to userspace application.
This commit confirms to return full identical information to the
operations.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
When event originator doesn't set numerical ID in identical information,
the event data includes no numerical ID, thus userspace applications
cannot identify the control just by unique ID in event data.
This commit fix this bug so as the event data includes all of identical
information.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
SNDRV_CTL_TLV_OP_XXX is defined but not used in core code. Instead,
raw numerical value is evaluated.
This commit replaces these values to these macros for better looking.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
In the commit [2225e79b9b03: 'ALSA: core: reduce stack usage related
to snd_ctl_new()'], the id field of the newly added kctl is untouched,
thus all attribute like name string remain empty. The fix is just to
add the forgotten memcpy of the id field.
Fixes: 2225e79b9b03 ('ALSA: core: reduce stack usage related to snd_ctl_new()')
Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
|
|
There was no check about the id string of user control elements, so we
accepted even a control element with an empty string, which is
obviously bogus. This patch adds more sanity checks of id strings.
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The kernel memory allocators already report the errors when the
requested allocation fails, thus we don't need to warn it again in
each caller side.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The callers of snd_ctl_new() need to have 'struct snd_kcontrol' data,
and pass the data as template. Then, the function allocates the structure
data again and copy from the template. This is a waste of resources.
Especially, the callers use large stack for the template.
This commit removes a need of template for the function, thus, changes
the prototype of snd_ctl_new(). Furthermore, this commit changes
the code of callers, snd_ctl_new1() and snd_ctl_elem_add() for better
shape.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The parameters can be decided in compile time.
This commit adds precomputed table to reduce calculating time.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
In 'replace' event data, numerical ID of control is always invalid. This
commit fix this bug so as the event data has renewed numerical ID for
control.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Currently when adding a new control, the assigned numerical ID is not
set for event data, thus userspace applications cannot realize it just
by event data.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Now that all callers have been replaced with
snd_device_register_for_dev(), let's drop the obsolete device
registration code and concentrate only on the code handling struct
device directly. That said,
- remove the old snd_device_register(),
- rename snd_device_register_for_dev() with snd_device_register(),
- drop superfluous arguments from snd_device_register(),
- change snd_unregister_device() to pass the device pointer directly
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
This patch embeds a struct device for the control device into the card
object and avoid the device creation at registration time.
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Instead of open-coding the search over the control file loop, provide
a helper function for the preferred subdevice assigned to the current
process.
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
We want to know the offset for the id that was passed to the function, not
the offset of the first id of the control (which is always 0).
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
A few functions have no proper documentation yet, so let's add them.
Along with it, remove superfluous blank line between the closing brace
and EXPORT_SYMBOL() line.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Although this is weird, some drivers want to allow empty control
elements intentionally, e.g. the number of items may change depending
on the firmware status. Let the function simply returning in such a
case.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
This allows us to catch the bugs in drivers easily.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Instead of hard-coded magic numbers, define constants for op_flag to
tlv callbacks.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The ALSA control code expects that the range of assigned indices to a control is
continuous and does not overflow. Currently there are no checks to enforce this.
If a control with a overflowing index range is created that control becomes
effectively inaccessible and unremovable since snd_ctl_find_id() will not be
able to find it. This patch adds a check that makes sure that controls with a
overflowing index range can not be created.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jaroslav Kysela <perex@perex.cz>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Each control gets automatically assigned its numids when the control is created.
The allocation is done by incrementing the numid by the amount of allocated
numids per allocation. This means that excessive creation and destruction of
controls (e.g. via SNDRV_CTL_IOCTL_ELEM_ADD/REMOVE) can cause the id to
eventually overflow. Currently when this happens for the control that caused the
overflow kctl->id.numid + kctl->count will also over flow causing it to be
smaller than kctl->id.numid. Most of the code assumes that this is something
that can not happen, so we need to make sure that it won't happen
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jaroslav Kysela <perex@perex.cz>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
A control that is visible on the card->controls list can be freed at any time.
This means we must not access any of its memory while not holding the
controls_rw_lock. Otherwise we risk a use after free access.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jaroslav Kysela <perex@perex.cz>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
There are two issues with the current implementation for replacing user
controls. The first is that the code does not check if the control is actually a
user control and neither does it check if the control is owned by the process
that tries to remove it. That allows userspace applications to remove arbitrary
controls, which can cause a user after free if a for example a driver does not
expect a control to be removed from under its feed.
The second issue is that on one hand when a control is replaced the
user_ctl_count limit is not checked and on the other hand the user_ctl_count is
increased (even though the number of user controls does not change). This allows
userspace, once the user_ctl_count limit as been reached, to repeatedly replace
a control until user_ctl_count overflows. Once that happens new controls can be
added effectively bypassing the user_ctl_count limit.
Both issues can be fixed by instead of open-coding the removal of the control
that is to be replaced to use snd_ctl_remove_user_ctl(). This function does
proper permission checks as well as decrements user_ctl_count after the control
has been removed.
Note that by using snd_ctl_remove_user_ctl() the check which returns -EBUSY at
beginning of the function if the control already exists is removed. This is not
a problem though since the check is quite useless, because the lock that is
protecting the control list is released between the check and before adding the
new control to the list, which means that it is possible that a different
control with the same settings is added to the list after the check. Luckily
there is another check that is done while holding the lock in snd_ctl_add(), so
we'll rely on that to make sure that the same control is not added twice.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jaroslav Kysela <perex@perex.cz>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The user-control put and get handlers as well as the tlv do not protect against
concurrent access from multiple threads. Since the state of the control is not
updated atomically it is possible that either two write operations or a write
and a read operation race against each other. Both can lead to arbitrary memory
disclosure. This patch introduces a new lock that protects user-controls from
concurrent access. Since applications typically access controls sequentially
than in parallel a single lock per card should be fine.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jaroslav Kysela <perex@perex.cz>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Use dev_err() & co as much as possible. If not available (no device
assigned at the calling point), use pr_xxx() helpers instead.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Replace the lengthy #if defined(XXX) || defined(XXX_MODULE) with the
new IS_ENABLED() macro.
The patch still doesn't cover all ifdefs. For example, the dependency
on CONFIG_GAMEPORT is still open-coded because this also has an extra
dependency on MODULE. Similarly, an open-coded ifdef in pcm_oss.c and
some sequencer-related stuff are left untouched.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
script/kernel-doc reports the following type of warnings (when run in verbose
mode):
Warning(sound/core/init.c:152): No description found for return value of
'snd_card_create'
To fix that:
- add missing descriptions of function return values
- use "Return:" sections to describe those return values
Along the way:
- complete some descriptions
- fix some typos
Signed-off-by: Yacine Belkadi <yacine.belkadi.1@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
When disconnect callback is called, each component should wake up
sleepers and check card->shutdown flag for avoiding the endless sleep
blocking the proper resource release.
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
For more strict protection for wild disconnections, a refcount is
introduced to the card instance, and let it up/down when an object is
referred via snd_lookup_*() in the open ops.
The free-after-last-close check is also changed to check this refcount
instead of the empty list, too.
Reported-by: Matthieu CASTET <matthieu.castet@parrot.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The SNDRV_CTL_ELEM_ACCESS_VOLATILE bit flag wasn't properly inherited
at creating control elements via snd_ctl_new1().
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Fixed a trailing white space error detected
in sound/core/control.c by checkpatch.pl script.
Signed-off-by: Jeffrin Jose <ahiliation@yahoo.co.in>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux
* 'modsplit-Oct31_2011' of git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux: (230 commits)
Revert "tracing: Include module.h in define_trace.h"
irq: don't put module.h into irq.h for tracking irqgen modules.
bluetooth: macroize two small inlines to avoid module.h
ip_vs.h: fix implicit use of module_get/module_put from module.h
nf_conntrack.h: fix up fallout from implicit moduleparam.h presence
include: replace linux/module.h with "struct module" wherever possible
include: convert various register fcns to macros to avoid include chaining
crypto.h: remove unused crypto_tfm_alg_modname() inline
uwb.h: fix implicit use of asm/page.h for PAGE_SIZE
pm_runtime.h: explicitly requires notifier.h
linux/dmaengine.h: fix implicit use of bitmap.h and asm/page.h
miscdevice.h: fix up implicit use of lists and types
stop_machine.h: fix implicit use of smp.h for smp_processor_id
of: fix implicit use of errno.h in include/linux/of.h
of_platform.h: delete needless include <linux/module.h>
acpi: remove module.h include from platform/aclinux.h
miscdevice.h: delete unnecessary inclusion of module.h
device_cgroup.h: delete needless include <linux/module.h>
net: sch_generic remove redundant use of <linux/module.h>
net: inet_timewait_sock doesnt need <linux/module.h>
...
Fix up trivial conflicts (other header files, and removal of the ab3550 mfd driver) in
- drivers/media/dvb/frontends/dibx000_common.c
- drivers/media/video/{mt9m111.c,ov6650.c}
- drivers/mfd/ab3550-core.c
- include/linux/dmaengine.h
|
|
This was introduced by 'ALSA: control: add support for ENUMERATED user
space controls' which adds a u64 variable that gets cast to a pointer:
sound/core/control.c: In function 'snd_ctl_elem_init_enum_names':
sound/core/control.c:1089: warning: cast to pointer from integer of different size
Cast to uintptr_t before casting to pointer to avoid the warning.
Signed-off-by: Olof Johansson <olof@lixom.net>
[cl: replace long with uintptr_t]
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Lots of sound drivers were getting module.h via the implicit presence
of it in <linux/device.h> but we are going to clean that up. So
fix up those users now.
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
|
|
Handling of user control elements was implemented for all types except
ENUMERATED. This type will be needed for the device-specific mixers of
upcoming FireWire drivers.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Suppose the ALSA card already has a number of MAX_USER_CONTROLS controls, and
the user wants to replace one, it should not fail at this condition check.
Signed-off-by: Lu Guanqun <guanqun.lu@intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
remove trailing tab on the line.
Signed-off-by: Lu Guanqun <guanqun.lu@intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Drop a few variables that are never read.
Signed-off-by: Luca Tettamanti <kronos.it@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Add a function to dynamically replace a given control. If the
control does not already exist, a third parameter is used to determine
whether to actually add that control. This is useful in cases where
downloadable firmware at runtime can add or replace existing controls.
A separate patch needs to be made to allow ALSA Mixer to render the
replaced controls on the fly.
Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
|
|
Added a new API function snd_ctl_activate_id() for activate / inactivate
the control element dynamically.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
|
