| Age | Commit message (Collapse) | Author |
|---|
|
commit c3ea60c231446663afd6ea1054da6b7f830855ca upstream.
There are two occurrances of a call to snd_seq_oss_fill_addr where
the dest_client and dest_port arguments are in the wrong order. Fix
this by swapping them around.
Addresses-Coverity: ("Arguments in wrong order")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
[ Upstream commit f0654ba94e33699b295ce4f3dc73094db6209035 ]
This reverts commit feb689025fbb6f0aa6297d3ddf97de945ea4ad32.
The fix attempt was incorrect, leading to the mutex deadlock through
the close of OSS sequencer client. The proper fix needs more
consideration, so let's revert it now.
Fixes: feb689025fbb ("ALSA: seq: Protect in-kernel ioctl calls with mutex")
Reported-by: syzbot+47ded6c0f23016cde310@syzkaller.appspotmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit 2eabc5ec8ab4d4748a82050dfcb994119b983750 ]
The snd_seq_ioctl_get_subscription() retrieves the port subscriber
information as a pointer, while the object isn't protected, hence it
may be deleted before the actual reference. This race was spotted by
syzkaller and may lead to a UAF.
The fix is simply copying the data in the lookup function that
performs in the rwsem to protect against the deletion.
Reported-by: syzbot+9437020c82413d00222d@syzkaller.appspotmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit feb689025fbb6f0aa6297d3ddf97de945ea4ad32 ]
ALSA OSS sequencer calls the ioctl function indirectly via
snd_seq_kernel_client_ctl(). While we already applied the protection
against races between the normal ioctls and writes via the client's
ioctl_mutex, this code path was left untouched. And this seems to be
the cause of still remaining some rare UAF as spontaneously triggered
by syzkaller.
For the sake of robustness, wrap the ioctl_mutex also for the call via
snd_seq_kernel_client_ctl(), too.
Reported-by: syzbot+e4c8abb920efa77bace9@syzkaller.appspotmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
commit 7c32ae35fbf9cffb7aa3736f44dec10c944ca18e upstream.
The call of unsubscribe_port() which manages the group count and
module refcount from delete_and_unsubscribe_port() looks racy; it's
not covered by the group list lock, and it's likely a cause of the
reported unbalance at port deletion. Let's move the call inside the
group list_mutex to plug the hole.
Reported-by: syzbot+e4c8abb920efa77bace9@syzkaller.appspotmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
commit 8c2f870890fd28e023b0fcf49dcee333f2c8bad7 upstream.
The ALSA proc helper manages the child nodes in a linked list, but its
addition and deletion is done without any lock. This leads to a
corruption if they are operated concurrently. Usually this isn't a
problem because the proc entries are added sequentially in the driver
probe procedure itself. But the card registrations are done often
asynchronously, and the crash could be actually reproduced with
syzkaller.
This patch papers over it by protecting the link addition and deletion
with the parent's mutex. There is "access" mutex that is used for the
file access, and this can be reused for this purpose as well.
Reported-by: syzbot+48df349490c36f9f54ab@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 2a3f7221acddfe1caa9ff09b3a8158c39b2fdeac upstream.
There is a small race window in the card disconnection code that
allows the registration of another card with the very same card id.
This leads to a warning in procfs creation as caught by syzkaller.
The problem is that we delete snd_cards and snd_cards_lock entries at
the very beginning of the disconnection procedure. This makes the
slot available to be assigned for another card object while the
disconnection procedure is being processed. Then it becomes possible
to issue a procfs registration with the existing file name although we
check the conflict beforehand.
The fix is simply to move the snd_cards and snd_cards_lock clearances
at the end of the disconnection procedure. The references to these
entries are merely either from the global proc files like
/proc/asound/cards or from the card registration / disconnection, so
it should be fine to shift at the very end.
Reported-by: syzbot+48df349490c36f9f54ab@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 212ac181c158c09038c474ba68068be49caecebb upstream.
When ioctl calls are made with non-null-terminated userspace strings,
strlcpy causes an OOB-read from within strlen. Fix by changing to use
strscpy instead.
Signed-off-by: Zubin Mithra <zsm@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
[ Upstream commit d9c0b2afe820fa3b3f8258a659daee2cc71ca3ef ]
BE dai links only have internal PCM's and their substream ops may
not be set. Suspending these PCM's will result in their
ops->trigger() being invoked and cause a kernel oops.
So skip suspending PCM's if their ops are NULL.
[ NOTE: this change is required now for following the recent PCM core
change to get rid of snd_pcm_suspend() call. Since DPCM BE takes
the runtime carried from FE while keeping NULL ops, it can hit this
bug. See details at:
https://github.com/thesofproject/linux/pull/582
-- tiwai ]
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
commit 113ce08109f8e3b091399e7cc32486df1cff48e7 upstream.
Currently PCM core sets each opened stream forcibly to SUSPENDED state
via snd_pcm_suspend_all() call, and the user-space is responsible for
re-triggering the resume manually either via snd_pcm_resume() or
prepare call. The scheme works fine usually, but there are corner
cases where the stream can't be resumed by that call: the streams
still in OPEN state before finishing hw_params. When they are
suspended, user-space cannot perform resume or prepare because they
haven't been set up yet. The only possible recovery is to re-open the
device, which isn't nice at all. Similarly, when a stream is in
DISCONNECTED state, it makes no sense to change it to SUSPENDED
state. Ditto for in SETUP state; which you can re-prepare directly.
So, this patch addresses these issues by filtering the PCM streams to
be suspended by checking the PCM state. When a stream is in either
OPEN, SETUP or DISCONNECTED as well as already SUSPENDED, the suspend
action is skipped.
To be noted, this problem was originally reported for the PCM runtime
PM on HD-audio. And, the runtime PM problem itself was already
addressed (although not intended) by the code refactoring commits
3d21ef0b49f8 ("ALSA: pcm: Suspend streams globally via device type PM
ops") and 17bc4815de58 ("ALSA: pci: Remove superfluous
snd_pcm_suspend*() calls"). These commits eliminated the
snd_pcm_suspend*() calls from the runtime PM suspend callback code
path, hence the racy OPEN state won't appear while runtime PM.
(FWIW, the race window is between snd_pcm_open_substream() and the
first power up in azx_pcm_open().)
Although the runtime PM issue was already "fixed", the same problem is
still present for the system PM, hence this patch is still needed.
And for stable trees, this patch alone should suffice for fixing the
runtime PM problem, too.
Reported-and-tested-by: Jon Hunter <jonathanh@nvidia.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
commit ca0214ee2802dd47239a4e39fb21c5b00ef61b22 upstream.
The PCM OSS emulation converts and transfers the data on the fly via
"plugins". The data is converted over the dynamically allocated
buffer for each plugin, and recently syzkaller caught OOB in this
flow.
Although the bisection by syzbot pointed out to the commit
65766ee0bf7f ("ALSA: oss: Use kvzalloc() for local buffer
allocations"), this is merely a commit to replace vmalloc() with
kvmalloc(), hence it can't be the cause. The further debug action
revealed that this happens in the case where a slave PCM doesn't
support only the stereo channels while the OSS stream is set up for a
mono channel. Below is a brief explanation:
At each OSS parameter change, the driver sets up the PCM hw_params
again in snd_pcm_oss_change_params_lock(). This is also the place
where plugins are created and local buffers are allocated. The
problem is that the plugins are created before the final hw_params is
determined. Namely, two snd_pcm_hw_param_near() calls for setting the
period size and periods may influence on the final result of channels,
rates, etc, too, while the current code has already created plugins
beforehand with the premature values. So, the plugin believes that
channels=1, while the actual I/O is with channels=2, which makes the
driver reading/writing over the allocated buffer size.
The fix is simply to move the plugin allocation code after the final
hw_params call.
Reported-by: syzbot+d4503ae45b65c5bc1194@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 c709f14f0616482b67f9fbcb965e1493a03ff30b upstream.
dev 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/seq/oss/seq_oss_synth.c:626 snd_seq_oss_synth_make_info() warn: potential spectre issue 'dp->synths' [w] (local cap)
Fix this by sanitizing dev before using it to index dp->synths.
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://lore.kernel.org/lkml/20180423164740.GY17484@dhcp22.suse.cz/
Cc: stable@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
commit 2b1d9c8f87235f593826b9cf46ec10247741fff9 upstream.
info->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/rawmidi.c:604 __snd_rawmidi_info_select() warn: potential spectre issue 'rmidi->streams' [r] (local cap)
Fix this by sanitizing info->stream before using it to index
rmidi->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://lore.kernel.org/lkml/20180423164740.GY17484@dhcp22.suse.cz/
Cc: stable@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
[ Upstream commit 678e2b44c8e3fec3afc7202f1996a4500a50be93 ]
The problem is seen in the q6asm_dai_compr_set_params() function:
ret = q6asm_map_memory_regions(dir, prtd->audio_client, prtd->phys,
(prtd->pcm_size / prtd->periods),
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
prtd->periods);
In this code prtd->pcm_size is the buffer_size and prtd->periods comes
from params->buffer.fragments. If we allow the number of fragments to
be zero then it results in a divide by zero bug. One possible fix would
be to use prtd->pcm_count directly instead of using the division to
re-calculate it. But I decided that it doesn't really make sense to
allow zero fragments.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
commit 94ffb030b6d31ec840bb811be455dd2e26a4f43e upstream.
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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
commit b51abed8355e5556886623b2772fa6b7598d2282 upstream.
Currently the PCM core calls snd_pcm_unlink() always unconditionally
at closing a stream. However, since snd_pcm_unlink() invokes the
global rwsem down, the lock can be easily contended. More badly, when
a thread runs in a high priority RT-FIFO, it may stall at spinning.
Basically the call of snd_pcm_unlink() is required only for the linked
streams that are already rare occasion. For normal use cases, this
code path is fairly superfluous.
As an optimization (and also as a workaround for the RT problem
above in normal situations without linked streams), this patch adds a
check before calling snd_pcm_unlink() and calls it only when needed.
Reported-by: Chanho Min <chanho.min@lge.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
commit b888a5f713e4d17faaaff24316585a4eb07f35b7 upstream.
Commit 67ec1072b053 ("ALSA: pcm: Fix rwsem deadlock for non-atomic PCM
stream") fixes deadlock for non-atomic PCM stream. But, This patch
causes antother stuck.
If writer is RT thread and reader is a normal thread, the reader
thread will be difficult to get scheduled. It may not give chance to
release readlocks and writer gets stuck for a long time if they are
pinned to single cpu.
The deadlock described in the previous commit is because the linux
rwsem queues like a FIFO. So, we might need non-FIFO writelock, not
non-block one.
My suggestion is that the writer gives reader a chance to be scheduled
by using the minimum msleep() instaed of spinning without blocking by
writer. Also, The *_nonblock may be changed to *_nonfifo appropriately
to this concept.
In terms of performance, when trylock is failed, this minimum periodic
msleep will have the same performance as the tick-based
schedule()/wake_up_q().
[ Although this has a fairly high performance penalty, the relevant
code path became already rare due to the previous commit ("ALSA:
pcm: Call snd_pcm_unlink() conditionally at closing"). That is, now
this unconditional msleep appears only when using linked streams,
and this must be a rare case. So we accept this as a quick
workaround until finding a more suitable one -- tiwai ]
Fixes: 67ec1072b053 ("ALSA: pcm: Fix rwsem deadlock for non-atomic PCM stream")
Suggested-by: Wonmin Jung <wonmin.jung@lge.com>
Signed-off-by: Chanho Min <chanho.min@lge.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
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 65766ee0bf7fe8b3be80e2e1c3ef54ad59b29476 upstream.
PCM OSS layer may allocate a few temporary buffers, one for the core
read/write and another for the conversions via plugins. Currently
both are allocated via vmalloc(). But as the allocation size is
equivalent with the PCM period size, the required size might be quite
small, depending on the application.
This patch replaces these vmalloc() calls with kvzalloc() for covering
small period sizes better. Also, we use "z"-alloc variant here for
addressing the possible uninitialized access reported by syzkaller.
Reported-by: syzbot+1cb36954e127c98dd037@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>
|
|
syzbot reported the uninitialized value exposure in certain situations
using virmidi loop. It's likely a very small race at writing and
reading, and the influence is almost negligible. But it's safer to
paper over this just by replacing the existing kvmalloc() with
kvzalloc().
Reported-by: syzbot+194dffdb8b22fc5d207a@syzkaller.appspotmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The recent change to move the virmidi output processing to a work
slightly modified the code to discard the unsubscribed outputs so that
it works without a temporary buffer. However, this is actually buggy,
and may spew a kernel warning due to the unexpected call of
snd_rawmidi_transmit_ack(), as triggered by syzbot.
This patch takes back to the original code in that part, use a
temporary buffer and simply repeat snd_rawmidi_transmit(), in order to
address the regression.
Fixes: f7debfe54090 ("ALSA: seq: virmidi: Offload the output event processing")
Reported-by: syzbot+ec5f605c91812d200367@syzkaller.appspotmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Warning level 2 was used: -Wimplicit-fallthrough=2
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Notice that in this particular case, I replaced the code comment with
a proper "fall through" annotation, which is what GCC is expecting
to find.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
For a sake of code simplification, remove the init and the exit
entries that do nothing.
Notes for readers: actually it's OK to remove *both* init and exit,
but not OK to remove the exit entry. By removing only the exit while
keeping init, the module becomes permanently loaded; i.e. you cannot
unload it any longer!
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The old ugly macros remained in the code without usage.
Rip them off.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
All usages of mutex in ALSA sequencer core would take too long, hence
we don't have to care about the user interruption that makes things
complicated. Let's replace them with simpler mutex_lock().
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The sequencer core module doesn't call some destructors in the error
path of the init code, which may leave some resources.
This patch mainly fix these leaks by calling the destructors
appropriately at alsa_seq_init(). Also the patch brings a few
cleanups along with it, namely:
- Expand the old "if ((err = xxx) < 0)" coding style
- Get rid of empty seq_queue_init() and its caller
- Change snd_seq_info_done() to void
Last but not least, a couple of functions lose __exit annotation since
they are called also in alsa_seq_init().
No functional changes but minor code cleanups.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
There are a few functions that have been commented out for ages.
And also there are functions that do nothing but placeholders.
Let's kill them.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
snd_midi_event_encode_byte() can never fail, and it can return rather
true/false. Change the return type to bool, adjust the argument to
receive a MIDI byte as unsigned char, and adjust the comment
accordingly. This allows callers to drop error checks, which
simplifies the code.
Meanwhile, snd_midi_event_encode() helper is used only in seq_midi.c,
and it can be better folded into it. This will reduce the total
amount of lines in the end.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Addresses-Coverity-ID: 1357375 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The trigger flag in vmidi object can be referred in different contexts
concurrently, hence it's better to be put with READ_ONCE() and
WRITE_ONCE() macros to assure the accesses.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The virmidi sequencer stuff tries to translate the rawmidi bytes to
sequencer events and deliver the packets at trigger callback. The
amount of the whole process of these translations and deliveries
depends on the incoming rawmidi bytes, and we have no limit for that;
this was the cause of a CPU soft lockup that had been reported and
fixed recently.
Although we've fixed the soft lockup by putting the temporary unlock
and cond_resched(), it's rather a quick band aid. In this patch,
meanwhile, the event parsing and delivery process is offloaded to a
dedicated work, and the trigger callback just kicks it off. It has
three merits, at least:
- The processing is always done in a sleepable context, which can
assure the event delivery with non-atomic flag without hackish
is_atomic() usage.
- Other relevant codes can be simplified, reducing the lines
- It makes me happier
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Pull the latest ALSA sequencer fixes for the further development of
virmidi.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The PCM format type is with __bitwise, hence it needs the explicit
cast with __force. It's ugly, but there is a reason for that cost...
This fixes the sparse warning:
sound/core/oss/pcm_oss.c:1854:55: warning: incorrect type in argument 1 (different base types)
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The virmidi output trigger tries to parse the all available bytes and
process sequencer events as much as possible. In a normal situation,
this is supposed to be relatively short, but a program may give a huge
buffer and it'll take a long time in a single spin lock, which may
eventually lead to a soft lockup.
This patch simply adds a workaround, a cond_resched() call in the loop
if applicable. A better solution would be to move the event processor
into a work, but let's put a duct-tape quickly at first.
Reported-and-tested-by: Dae R. Jeong <threeearcat@gmail.com>
Reported-by: syzbot+619d9f40141d826b097e@syzkaller.appspotmail.com
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Instead of open codes, use the standard macros for obtaining the lower
and upper 32bit values.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The sanity checks in ALSA sequencer and OSS sequencer emulation codes
return falsely -ENXIO from poll callback. They should be EPOLLERR
instead.
This was caught thanks to the recent change to the return value.
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
snd_dma_alloc_pages_fallback() tries to allocate pages again when the
allocation fails with reduced size. But the first try actually
*increases* the size to power-of-two, which may give back a larger
chunk than the requested size. This confuses the callers, e.g. sgbuf
assumes that the size is equal or less, and it may result in a bad
loop due to the underflow and eventually lead to Oops.
The code of this function seems incorrectly assuming the usage of
get_order(). We need to decrease at first, then align to
power-of-two.
Reported-and-tested-by: he, bo <bo.he@intel.com>
Reported-by: zhang jun <jun.zhang@intel.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
A timer object for the classes SNDRV_TIMER_CLASS_CARD and
SNDRV_TIMER_CLASS_PCM has to be associated with a card object, but we
have no check at creation time. Such a timer object with NULL card
causes various unexpected problems, e.g. NULL dereference at reading
the sound timer proc file.
So as preventive measure while the creating the sound timer object is
created the card information availability is checked for the mentioned
entries and returned error if its NULL.
Signed-off-by: Srikanth K H <srikanth.h@samsung.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The size of in-kernel rawmidi buffers may be big up to 1MB, and it can
be specified freely by user-space; which implies that user-space may
trigger kmalloc() errors frequently.
This patch replaces the buffer allocation via kvmalloc() for dealing
with bigger buffers gracefully.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Unify a few open codes with helper functions to improve the
readability. Minor behavior changes (rather fixes) are:
- runtime->drain clearance is done within lock
- active_sensing is updated before resizing buffer in
SNDRV_RAWMIDI_IOCTL_PARAMS ioctl.
Other than that, simply code cleanups.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Apply the standard idiom: rewrite the multiple unlocks in error paths
in the goto-error-and-single-unlock way.
Just a code refactoring, and no functional changes.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Just minor coding style fixes like removal of superfluous white space,
adding missing blank lines, etc. No actual code changes at all.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Back-merge for further cleanup / improvements on rawmidi and HD-audio
stuff.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The SNDRV_RAWMIDI_IOCTL_PARAMS ioctl may resize the buffers and the
current code is racy. For example, the sequencer client may write to
buffer while it being resized.
As a simple workaround, let's switch to the resized buffer inside the
stream runtime lock.
Reported-by: syzbot+52f83f0ea8df16932f7f@syzkaller.appspotmail.com
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Without this commit the following intervals [x y), (x y) were be
replaced to (y-1 y) by snd_interval_refine_last(). This was also done
if y-1 is part of the previous interval.
With this changes it will be replaced with [y-1 y) in case of y-1 is
part of the previous interval. A similar behavior will be used for
snd_interval_refine_first().
This commit adapts the changes for alsa-lib of commit
9bb985c ("pcm: snd_interval_refine_first/last: exclude value only if
also excluded before")
Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Currently ALSA core blocks userspace for about 10 seconds for PCM R/W IO.
This needs to be configurable for modern hardware like DSPs where no
pointer update in milliseconds can indicate terminal DSP errors.
Add a substream variable to set the wait time in ms. This allows userspace
and drivers to recover more quickly from terminal DSP errors.
Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
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>
|
|
The kernel may spew a WARNING with UBSAN undefined behavior at
handling ALSA sequencer ioctl SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT:
UBSAN: Undefined behaviour in sound/core/seq/seq_clientmgr.c:2007:14
signed integer overflow:
2147483647 + 1 cannot be represented in type 'int'
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x122/0x1c8 lib/dump_stack.c:113
ubsan_epilogue+0x12/0x86 lib/ubsan.c:159
handle_overflow+0x1c2/0x21f lib/ubsan.c:190
__ubsan_handle_add_overflow+0x2a/0x31 lib/ubsan.c:198
snd_seq_ioctl_query_next_client+0x1ac/0x1d0 sound/core/seq/seq_clientmgr.c:2007
snd_seq_ioctl+0x264/0x3d0 sound/core/seq/seq_clientmgr.c:2144
....
It happens only when INT_MAX is passed there, as we're incrementing it
unconditionally. So the fix is trivial, check the value with
INT_MAX. Although the bug itself is fairly harmless, it's better to
fix it so that fuzzers won't hit this again later.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=200211
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
