The read from ALSA timer device, the function snd_timer_user_tread(),
may access to an uninitialized struct snd_timer_user fields when the
read is concurrently performed while the ioctl like
snd_timer_user_tselect() is invoked. We have already fixed the races
among ioctls via a mutex, but we seem to have forgotten the race
between read vs ioctl.
This patch simply applies (more exactly extends the already applied
range of) tu->ioctl_lock in snd_timer_user_tread() for closing the
race window.
Change-Id: I3b539d6e01d86d5b9cbe813e2616894e6202225f
Reported-by: Alexander Potapenko <glider@google.com>
Tested-by: Alexander Potapenko <glider@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
snd_timer_user_tselect() reallocates the queue buffer dynamically, but
it forgot to reset its indices. Since the read may happen
concurrently with ioctl and snd_timer_user_tselect() allocates the
buffer via kmalloc(), this may lead to the leak of uninitialized
kernel-space data, as spotted via KMSAN:
BUG: KMSAN: use of unitialized memory in snd_timer_user_read+0x6c4/0xa10
CPU: 0 PID: 1037 Comm: probe Not tainted 4.11.0-rc5+ #2739
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:16
dump_stack+0x143/0x1b0 lib/dump_stack.c:52
kmsan_report+0x12a/0x180 mm/kmsan/kmsan.c:1007
kmsan_check_memory+0xc2/0x140 mm/kmsan/kmsan.c:1086
copy_to_user ./arch/x86/include/asm/uaccess.h:725
snd_timer_user_read+0x6c4/0xa10 sound/core/timer.c:2004
do_loop_readv_writev fs/read_write.c:716
__do_readv_writev+0x94c/0x1380 fs/read_write.c:864
do_readv_writev fs/read_write.c:894
vfs_readv fs/read_write.c:908
do_readv+0x52a/0x5d0 fs/read_write.c:934
SYSC_readv+0xb6/0xd0 fs/read_write.c:1021
SyS_readv+0x87/0xb0 fs/read_write.c:1018
This patch adds the missing reset of queue indices. Together with the
previous fix for the ioctl/read race, we cover the whole problem.
Change-Id: I5143563a56255d4063992e75f360972658b3eb21
Reported-by: Alexander Potapenko <glider@google.com>
Tested-by: Alexander Potapenko <glider@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Pointer after kfree is not sanitized.
Set pointer to NULL.
CRs-Fixed: 2008031
Change-Id: Ia59a57fcd142a6ed18d168992b8da4019314afa4
Signed-off-by: Xiaojun Sang <xsang@codeaurora.org>
Signed-off-by: Bikshapathi Kothapeta <bkotha@codeaurora.org>
ALSA sequencer code has an open race between the timer setup ioctl and
the close of the client. This was triggered by syzkaller fuzzer, and
a use-after-free was caught there as a result.
This patch papers over it by adding a proper queue->timer_mutex lock
around the timer-related calls in the relevant code path.
Change-Id: I58773978b545fb73311e2eab973ab63c9099dba1
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Tested-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
I previously added an integer overflow check here but looking at it now,
it's still buggy.
The bug happens in snd_compr_allocate_buffer(). We multiply
".fragments" and ".fragment_size" and that doesn't overflow but then we
save it in an unsigned int so it truncates the high bits away and we
allocate a smaller than expected size.
Fixes: b35cc82258 ('ALSA: compress_core: integer overflow in snd_compr_allocate_buffer()')
Change-Id: I7759b19003e8bebbcb16652a6398a8ef8c355250
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Userspace registers calibration data with acdb driver
through ioctls. Voice driver registers the calibration
data with CVD by querying acdb data from acdb driver and
copies the calibration data in apr message.
The size of the calibration data can be controlled by userspace
and can result in buffer overflow if the calibration size is
greater than the destination buffer size.
Reject acdb data if the size is greater than the size of
destination buffer.
Change-Id: I1b495390d67907afc608ed8233bf916777d38e11
CRs-fixed: 548872
Signed-off-by: Shiv Maliyappanahalli <smaliyap@codeaurora.org>
snd_info_entry_write() resizes the buffer with an unsigned long
size argument that gets truncated because resize_info_buffer()
takes the size parameter as an unsigned int. On 64-bit kernels,
this causes the following copy_to_user() to write out-of-bounds
if (pos + count) can't be represented by an unsigned int.
Bug: 32510733
Change-Id: I9e8b55f93f2bd606b4a73b5a4525b71ee88c7c23
Signed-off-by: Siqi Lin <siqilin@google.com>
A copy_from_user is not always expected to succeed. Therefore, check
for an error before operating on the buffer post copy.
CAF-Change-Id: Ibba9a47c84e735d30e32eeac5b80d51044b7a9e8
CRs-Fixed: 1094852
Signed-off-by: Siena Richard <sienar@codeaurora.org>
CVE-2017-0610
Change-Id: I238dd28d531778104660ff5ac9b8a9733c2fac6a
(cherry picked from commit 65009746a6e649779f73d665934561ea983892fe)
Range checking is added to prevent buffer overflow that due to inputs
can be set by user space.
CRs-Fixed: 1098363
CAF-Change-Id: I057261291806240ee6d7b8106a5e83a7665e013d
Signed-off-by: Xiaoyu Ye <benyxy@codeaurora.org>
CVE-2017-0608
Change-Id: I41aee14edf5a7dd257aae78132047048b82b127e
(cherry picked from commit b66f442dd97c781e873e8f7b248e197f86fd2980)
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>
CVE-2014-4656
Change-Id: Id984d11f78449f476804642ccfaf09380ad70ac9
(cherry picked from commit 883a1d49f0)
Currently kill_fasync() is called outside the stream lock in
snd_pcm_period_elapsed(). This is potentially racy, since the stream
may get released even during the irq handler is running. Although
snd_pcm_release_substream() calls snd_pcm_drop(), this doesn't
guarantee that the irq handler finishes, thus the kill_fasync() call
outside the stream spin lock may be invoked after the substream is
detached, as recently reported by KASAN.
As a quick workaround, move kill_fasync() call inside the stream
lock. The fasync is rarely used interface, so this shouldn't have a
big impact from the performance POV.
Ideally, we should implement some sync mechanism for the proper finish
of stream and irq handler. But this oneliner should suffice for most
cases, so far.
Reported-by: Baozeng Ding <sploving1@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
(cherry picked from commit 3aa02cb664c5fb1042958c8d1aa8c35055a2ebc4)
Change-Id: I921b3b0b4a7dfaa6267df71676d99e8dc2fb303f
While there is nothing wrong with the transfer_ack_begin and
transfer_ack_end callbacks per-se, the last documented user was part of the
alsa-driver 0.5.12a package, which was released 14 years ago and even
predates the upstream integration of the ALSA core and has subsequently
been superseded by newer alsa-driver releases.
This seems to indicate that there is no need for having these callbacks and
they are just cruft that can be removed.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
(cherry picked from commit 53e597b1d194910bef53ed0632da329fef497904)
Change-Id: Ifa69c873640b171aa1843335b2b3cb856d29bb1a
create_fixed_stream_quirk(), snd_usb_parse_audio_interface() and
create_uaxx_quirk() functions allocate the audioformat object by themselves
and free it upon error before returning. However, once the object is linked
to a stream, it's freed again in snd_usb_audio_pcm_free(), thus it'll be
double-freed, eventually resulting in a memory corruption.
This patch fixes these failures in the error paths by unlinking the audioformat
object before freeing it.
Based on a patch by Takashi Iwai <tiwai@suse.de>
[Note for stable backports:
this patch requires the commit 902eb7fd1e4a ('ALSA: usb-audio: Minor
code cleanup in create_fixed_stream_quirk()')]
Change-Id: I129dc4f3b0ae4cb6f790c16d24dd768c9ee06822
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1283358
Reported-by: Ralf Spenneberg <ralf@spenneberg.net>
Cc: <stable@vger.kernel.org> # see the note above
Signed-off-by: Vladis Dronov <vdronov@redhat.com>
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.
Change-Id: Ia4bd6bff33e86ee8b971031381d07b80bd383171
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>
hrtimer_cancel() waits for the completion from the callback, thus it
must not be called inside the callback itself. This was already a
problem in the past with ALSA hrtimer driver, and the early commit
[fcfdebe707: ALSA: hrtimer - Fix lock-up] tried to address it.
However, the previous fix is still insufficient: it may still cause a
lockup when the ALSA timer instance reprograms itself in its callback.
Then it invokes the start function even in snd_timer_interrupt() that
is called in hrtimer callback itself, results in a CPU stall. This is
no hypothetical problem but actually triggered by syzkaller fuzzer.
This patch tries to fix the issue again. Now we call
hrtimer_try_to_cancel() at both start and stop functions so that it
won't fall into a deadlock, yet giving some chance to cancel the queue
if the functions have been called outside the callback. The proper
hrtimer_cancel() is called in anyway at closing, so this should be
enough.
Change-Id: Id6224b2a3ade0d217e891e6af09744df4d0b2e5c
Reported-and-tested-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
A slave timer instance might be still accessible in a racy way while
operating the master instance as it lacks of locking. Since the
master operation is mostly protected with timer->lock, we should cope
with it while changing the slave instance, too. Also, some linked
lists (active_list and ack_list) of slave instances aren't unlinked
immediately at stopping or closing, and this may lead to unexpected
accesses.
This patch tries to address these issues. It adds spin lock of
timer->lock (either from master or slave, which is equivalent) in a
few places. For avoiding a deadlock, we ensure that the global
slave_active_lock is always locked at first before each timer lock.
Also, ack and active_list of slave instances are properly unlinked at
snd_timer_stop() and snd_timer_close().
Last but not least, remove the superfluous call of _snd_timer_stop()
at removing slave links. This is a noop, and calling it may confuse
readers wrt locking. Further cleanup will follow in a later patch.
Actually we've got reports of use-after-free by syzkaller fuzzer, and
this hopefully fixes these issues.
Change-Id: I572878b909dda522dbedc84633414185802bc974
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
ALSA timer instance object has a couple of linked lists and they are
unlinked unconditionally at snd_timer_stop(). Meanwhile
snd_timer_interrupt() unlinks it, but it calls list_del() which leaves
the element list itself unchanged. This ends up with unlinking twice,
and it was caught by syzkaller fuzzer.
The fix is to use list_del_init() variant properly there, too.
Change-Id: I95e2ab06180dfe43fb6b7c2875a866b53ca245ce
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Tested-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The 'umidi' object will be free'd on the error path by snd_usbmidi_free()
when tearing down the rawmidi interface. So we shouldn't try to free it
in snd_usbmidi_create() after having registered the rawmidi interface.
Found by KASAN.
Change-Id: I8534867beeac111370017ef246adc17e23e1a3b1
Signed-off-by: Andrey Konovalov <andreyknvl@gmail.com>
Acked-by: Clemens Ladisch <clemens@ladisch.de>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The stack object “r1” has a total size of 32 bytes. Its field
“event” and “val” both contain 4 bytes padding. These 8 bytes
padding bytes are sent to user without being initialized.
Bug: 28980217
Change-Id: Iff69ca708e0022ce9301efae798798b9bfcf9e25
Signed-off-by: Kangjie Lu <kjlu@gatech.edu>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Siqi Lin <siqilin@google.com>
(cherry picked from commit 9a47e9cff994f37f7f0dbd9ae23740d0f64f9fe6)
The stack object “r1” has a total size of 32 bytes. Its field
“event” and “val” both contain 4 bytes padding. These 8 bytes
padding bytes are sent to user without being initialized.
Bug: 28980217
Change-Id: I2bef279bbaa1f20ea831d364b3a4a09a27f07025
Signed-off-by: Kangjie Lu <kjlu@gatech.edu>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Siqi Lin <siqilin@google.com>
(cherry picked from commit e4ec8cc8039a7063e24204299b462bd1383184a5)
The stack object “tread” has a total size of 32 bytes. Its field
“event” and “val” both contain 4 bytes padding. These 8 bytes
padding bytes are sent to user without being initialized.
Bug: 28980557
Change-Id: Ib66cfcc1e36025255d7f518f3df2c39a21858886
Signed-off-by: Kangjie Lu <kjlu@gatech.edu>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Siqi Lin <siqilin@google.com>
(cherry picked from commit cec8f96e49d9be372fdb0c3836dcf31ec71e457e)
ALSA timer ioctls have an open race and this may lead to a
use-after-free of timer instance object. A simplistic fix is to make
each ioctl exclusive. We have already tread_sem for controlling the
tread, and extend this as a global mutex to be applied to each ioctl.
The downside is, of course, the worse concurrency. But these ioctls
aren't to be parallel accessible, in anyway, so it should be fine to
serialize there.
Bug: 28694392
Change-Id: I1ac52f1cba5e7408fd88c8fc1c30ca2e83967ebb
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Tested-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Siqi Lin <siqilin@google.com>
(cherry picked from commit af368027a49a751d6ff4ee9e3f9961f35bb4fede)
The overflow check is required to ensure that user space data
in kernel may not go beyond buffer boundary.
Bug: 28751152
Change-Id: I79b7e5f875fadcaeceb05f9163ae3666d4b6b7e1
CRs-Fixed: 563086
Signed-off-by: Mohammad Johny Shaik <mjshai@codeaurora.org>
snd_compr_tstamp is initialized using aggregate initialization
that does not zero out the padded bytes. Initialize timestamp
structure to zero using memset to avoid this.
Bug: 28770164
CRs-Fixed: 568717
Change-Id: I7a7d188705161f06201f1a1f2945bb6acd633d5d
Signed-off-by: Krishnankutty Kolathappilly <kkolat@codeaurora.org>
Headset mic fail in case of inserting headset while dmic is recording.
Keep LDO_H always on while headset is inserted.
Bug: 11506684
Change-Id: I8516d537d2c72d6f71236219e5d3e610e25ecf24
Signed-off-by: sam_chen <sam_chen@asus.com>
During build-in mic recording, insert or remove headphone would
cause LDO_H power down which leads to recording fail.
Power on or down LDO_H by checking dapm widget LDO_H status.
Bug: 11523570
Change-Id: Ib7558748c093b60830eb41b2171c2eae95e4ed0a
Signed-off-by: sam_chen <sam_chen@asus.com>
Always disable micbias when headphone insertion to save power around 1.5mA.
Bug: 9946473
Change-Id: I7cd2df4872b8388287df69a344f1dc1d45653405
Signed-off-by: Karl Yu <Karl_Yu@asus.com>
Support audio control for one-button headsets.
Also refine code formats.
Bug: 9196319
Change-Id: Id572bb86dcefd52ea204c60bf4fda1e6c02fd135
Signed-off-by: ChungYi_Guan <ChungYi_Guan@asus.com>
Reduce debouncing time for unplug cases according to EE measurement.
Bug: 9083368
Change-Id: I9025f852beb69ece7f85863f86388e833e4ef64a
Signed-off-by: ChungYi_Guan <ChungYi_Guan@asus.com>
D-mic Vdd is connected to micbias1.
To set micbias1 to 1.8V, we need to set LDO_H_1 output power to 2.85V
which is the source of micbias1.
Bug: 9042676
Change-Id: I222ac01c24346031d25483e80f554a4dc0833c43
Signed-off-by: sam_chen <sam_chen@asus.com>
Compress core added metadata apis in 9727b4, so add same in ASoC
Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Fix compilation error after merging commit 00642f5
from master: pop_wait is in struct snd_soc_dai
not in struct snd_soc_pcm_runtime.
Signed-off-by: Eric Laurent <elaurent@google.com>
When a new stream is being opened it is necessary to cancel any delayed
power down of the audio.
[Fixed unused variable -- broonie]
Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
There were variations in the time taken to
return from each write. This was due
to the delay in switching in and out of
power collapse. To manage this set QOS
value to 1ms so that the core has enough
time to wake up from power collapse
Signed-off-by: Alexy Joseph <alexyj@codeaurora.org>
Signed-off-by: Mekala Natarajan <mekalan@codeaurora.org>
On flo hardware revision C, micbias1 is not grounded with external
capacity, so it should set micbias1 capless setting as 1
(no external bypass capacity) to avoid noise.
Bug:8611206
Change-Id: I82644a9123d092490ccc0acf6cdfa68964ef9c22
Signed-off-by: sam_chen <sam_chen@asus.com>
Otherwise capture activity on a compressed DAI would mute any playback
on the same DAI.
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Acked-by: Vinod Koul <vinod.koul@intel.com>
Acked-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Fix compilation error after merging commit 1f88eb0
from master: pop_wait is in struct snd_soc_dai
not in struct snd_soc_pcm_runtime.
Signed-off-by: Eric Laurent <elaurent@google.com>
The ASoC compressed API did not implement the copy callback in its
compressed ops which is required for DSPs that are not memory mapped.
This patch creates a local copy of the compress ops for each runtime and
modifies them with a copy callback as appropriate.
Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Acked-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Delayed work was scheduled but not initialised, this patch adds the
actual work and initialises it.
Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Acked-by: Vinod Koul <vinod.koul@intel.com>
Tested-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Use the pcm_mutex to serialise the compressed ops.
Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Acked-by: Vinod Koul <vinod.koul@intel.com>
Tested-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Fix compilation error after merging commit 1245b700
from master: snd_soc_dapm_stream_event() protoype has changed.
Signed-off-by: Eric Laurent <elaurent@google.com>
This patch adds the support to parse the compress dai's and then also adds the
soc-compress.c file while handles the compress stream operations, mostly analogus
to what is done in the soc-pcm.c and aditional handling of the compress
opertaions
Conflicts:
sound/soc/soc-core.c
Signed-off-by: Namarta Kohli <namartax.kohli@intel.com>
Signed-off-by: Ramesh Babu K V <ramesh.babu@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@linux.intel.com>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
this add new API for sound compress to support gapless playback.
As noted in Documentation change, we add API to send metadata of encoder and
padding delay to DSP. Also add API for indicating EOF and switching to
subsequent track
Also bump the compress API version
Conflicts:
include/uapi/sound/compress_offload.h
Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The snd_compr_update_tstamp() can only fill in the snd_compr_tstamp
if the codec implements the pointer() function. If that happened
the code was previously returning uninitialized garbage in the
tstamp because it wasn't initialized anywhere.
This change zero-fills the tstamp in the two places it is used
before calling snd_compr_update_tstamp(), and also has
snd_compr_update_tstamp() return an error indication if it
can't provide a tstamp. For the case of snd_compr_calc_avail()
it ignores this error because we still need to return info on
the available buffer space even if we can't provide tstamp
info - when the tstamp is not valid all fields are now
guaranteed to be zero.
Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Acked-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
