Factor out the common codes in snd_pcm_*_forward() and *_rewind()
functions to simplify the codes. No functional changes.
Reviewd-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The mostly same codes for checking the current PCM state and calling
hwsync are found in a few places. This patch simplifies them by
creating a common helper function.
It also fixes a couple of cases where we missed the proper state check
(e.g. PAUSED state wasn't handled in rewind and snd_pcm_hwsync()),
too.
Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Only one of the two declarations has the const modifier in the
argument list, so we get a warning when CONFIG_SND_PCM_OSS_PLUGINS
is disabled:
sound/core/oss/pcm_oss.c: In function 'snd_pcm_oss_change_params':
sound/core/oss/pcm_oss.c:898:47: error: passing argument 2 of 'snd_pcm_plug_slave_format' discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
This makes the two declarations match again.
Fixes: e76bf3c4b4 ("ALSA: pcm/oss: refer to parameters instead of copying to reduce usage of kernel stack")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-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: 58f30d650c ("ALSA: pcm: Build pcm notifier code conditionally")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
In a function snd_pcm_hw_params_choose(), target parameters are arranged
into a table. Though each entry of this table is read-only, they don't
have const qualifier.
This commit adds the qualifier.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Some functions in compatibility layer for Open Sound System interface has
local variable to copy some parameters in runtime of PCM substream, while
this can be replaced with reference of pointers to parameter itself. This
brings an advantage to reduce usage of kernel stack.
This commit applies this idea.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
ALSA pcm core has hw_param_interval_c() to pick up parameter with const
qualifier for safe programming.
This commit applies it to the cases.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
There's a read-only table for each sampling rate, while it doesn't have
const qualifier and can be modified.
This commit add the qualifier. As a result, a symbol for the table
moves from .data section to .rodata.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
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 snd_use_lock_sync() (thus its implementation
snd_use_lock_sync_helper()) has the 5 seconds timeout to break out of
the sync loop. It was introduced from the beginning, just to be
"safer", in terms of avoiding the stupid bugs.
However, as Ben Hutchings suggested, this timeout rather introduces a
potential leak or use-after-free that was apparently fixed by the
commit 2d7d54002e ("ALSA: seq: Fix race during FIFO resize"):
for example, snd_seq_fifo_event_in() -> snd_seq_event_dup() ->
copy_from_user() could block for a long time, and snd_use_lock_sync()
goes timeout and still leaves the cell at releasing the pool.
For fixing such a problem, we remove the break by the timeout while
still keeping the warning.
Suggested-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The "r1" struct has memory holes. We clear it with memset on one path
where it is used but not the other. Let's just memset it at the start
of the function so it's always safe.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
We just checked "id.card < 0" on the lines before so we know it's not
true here. We can delete that check.
Also checkpatch.pl complains about some extra curly braces so we may as
well fix that while we're at it.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
When a new event is queued while processing to resize the FIFO in
snd_seq_fifo_clear(), it may lead to a use-after-free, as the old pool
that is being queued gets removed. For avoiding this race, we need to
close the pool to be deleted and sync its usage before actually
deleting it.
The issue was spotted by syzkaller.
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
When snd_seq_pool_done() is called, it marks the closing flag to
refuse the further cell insertions. But snd_seq_pool_done() itself
doesn't clear the cells but just waits until all cells are cleared by
the caller side. That is, it's racy, and this leads to the endless
stall as syzkaller spotted.
This patch addresses the racy by splitting the setup of pool->closing
flag out of snd_seq_pool_done(), and calling it properly before
snd_seq_pool_done().
BugLink: http://lkml.kernel.org/r/CACT4Y+aqqy8bZA1fFieifNxR2fAfFQQABcBHj801+u5ePV0URw@mail.gmail.com
Reported-and-tested-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Fix up affected files that include this signal functionality via sched.h.
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
A few last-minute fixes for rc1:
- ALSA core timer and sequencer fixes for bugs spotted by syzkaller
- A couple of trivial HD-audio fixups
- Additional PCI / codec IDs for Intel Geminilake
- Fixes for CT-XFi DMA mask bugs
-----BEGIN PGP SIGNATURE-----
iQJCBAABCAAsFiEECxfAB4MH3rD5mfB6bDGAVD0pKaQFAli2mQcOHHRpd2FpQHN1
c2UuZGUACgkQbDGAVD0pKaTboA//fvaaaGlU+IUBvEHV8LlqyfHOUBIIyowYIeJE
mQHV4lrdMa9vEfXpIB5bztrqRcB6O9Trw0l4gEZfkC+YgLdZKZXi/haSdVnYsHRi
koD8ZM1EtNu8o9FzGhOg8Cefu+bvQCg/dAglt6oitf3av/j6NmpFwC3EZzcAosH/
VE3VRBk8AyQj1DYmlfoc+i27ksO9OceQI9xvJvGwdbbwzBTH/dL+PGXCwfF88T2p
CyUvxtCk2HohHMloV6PtbpdD+ldouZxRvQsiV6MRy0Wg+ARAILvjeS9gdn3UT2LE
E5J2JLM1X0x5J82Hki3z9vctwvmZbifCj/ewlql+3gFgAAvt0/PiRYZ0W1jcUpK6
5THLRwU8zCOuAhQxsEzhDZh6mQq/gV69mWdVCgp3Er5faZQm6LqPUsHp2+yB/0aK
0mXFRCAIjJa62ddtl40LPkPtJoEX9M00+ILNeASjMhpZSM8KyuBUnqygCoB1Kxhv
vOiNhfIzXl8wQl406o/nIiDzbOdK7Ze7GyT6DHNQtNaS/aA9lS5RxYfcxDGce6c4
nm9bnfkvRypNeY+dQwX5KefOd+ilYLHcOevUv1rC395pby7rEuVuMl/j83Qr8Pof
cIjdSFefCCCQafH37UZhQ31noBIrxNFwlDBJh2YLWVDj6tU5ikY4GnjZQnMX/uv9
/6XhpiM=
=hOYM
-----END PGP SIGNATURE-----
Merge tag 'sound-fix-4.11-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
Pull sound fixes from Takashi Iwai:
"A few last-minute fixes for rc1:
- ALSA core timer and sequencer fixes for bugs spotted by syzkaller
- a couple of trivial HD-audio fixups
- additional PCI / codec IDs for Intel Geminilake
- fixes for CT-XFi DMA mask bugs"
* tag 'sound-fix-4.11-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound:
ALSA: seq: Fix link corruption by event error handling
ALSA: hda - Add subwoofer support for Dell Inspiron 17 7000 Gaming
ALSA: ctxfi: Fallback DMA mask to 32bit
ALSA: timer: Reject user params with too small ticks
ALSA: hda: Add Geminilake HDMI codec ID
ALSA: hda - Fix micmute hotkey problem for a lenovo AIO machine
ALSA: hda - Add Geminilake PCI ID
The sequencer FIFO management has a bug that may lead to a corruption
(shortage) of the cell linked list. When a sequencer client faces an
error at the event delivery, it tries to put back the dequeued cell.
When the first queue was put back, this forgot the tail pointer
tracking, and the link will be screwed up.
Although there is no memory corruption, the sequencer client may stall
forever at exit while flushing the pending FIFO cells in
snd_seq_pool_done(), as spotted by syzkaller.
This patch addresses the missing tail pointer tracking at
snd_seq_fifo_cell_putback(). Also the patch makes sure to clear the
cell->enxt pointer at snd_seq_fifo_event_in() for avoiding a similar
mess-up of the FIFO linked list.
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
When a user sets a too small ticks with a fine-grained timer like
hrtimer, the kernel tries to fire up the timer irq too frequently.
This may lead to the condensed locks, eventually the kernel spinlock
lockup with warnings.
For avoiding such a situation, we define a lower limit of the
resolution, namely 1ms. When the user passes a too small tick value
that results in less than that, the kernel returns -EINVAL now.
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
->fault(), ->page_mkwrite(), and ->pfn_mkwrite() calls do not need to
take a vma and vmf parameter when the vma already resides in vmf.
Remove the vma parameter to simplify things.
[arnd@arndb.de: fix ARM build]
Link: http://lkml.kernel.org/r/20170125223558.1451224-1-arnd@arndb.de
Link: http://lkml.kernel.org/r/148521301778.19116.10840599906674778980.stgit@djiang5-desk3.ch.intel.com
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Theodore Ts'o <tytso@mit.edu>
Cc: Darrick J. Wong <darrick.wong@oracle.com>
Cc: Matthew Wilcox <mawilcox@microsoft.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Jan Kara <jack@suse.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
When a sequencer queue is created in snd_seq_queue_alloc(),it adds the
new queue element to the public list before referencing it. Thus the
queue might be deleted before the call of snd_seq_queue_use(), and it
results in the use-after-free error, as spotted by syzkaller.
The fix is to reference the queue object at the right time.
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
snd_seq_pool_done() syncs with closing of all opened threads, but it
aborts the wait loop with a timeout, and proceeds to the release
resource even if not all threads have been closed. The timeout was 5
seconds, and if you run a crazy stuff, it can exceed easily, and may
result in the access of the invalid memory address -- this is what
syzkaller detected in a bug report.
As a fix, let the code graduate from naiveness, simply remove the loop
timeout.
BugLink: http://lkml.kernel.org/r/CACT4Y+YdhDV2H5LLzDTJDVF-qiYHUHhtRaW4rbb4gUhTCQB81w@mail.gmail.com
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Now snd_rawmidi_ops is maintained as a const pointer in snd_rawmidi,
we can constify the definitions.
Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Make snd_rawmidi_substream.ops to be a const pointer to be safer and
allow more optimization. The patches to constify each rawmidi ops
will follow.
Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
ktime is a union because the initial implementation stored the time in
scalar nanoseconds on 64 bit machine and in a endianess optimized timespec
variant for 32bit machines. The Y2038 cleanup removed the timespec variant
and switched everything to scalar nanoseconds. The union remained, but
become completely pointless.
Get rid of the union and just keep ktime_t as simple typedef of type s64.
The conversion was done with coccinelle and some manual mopping up.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
No dramatic changes are found in this development cycle, but as usual,
many commits are applied in a wide range of drivers.
Most of big changes are in ASoC, where a few bits of framework work
and quite a lot of cleanups and improvements to existing code have
been done. The rest are usual stuff, a few HD-audio and USB-audio
quirks and fixes, as well as the drop of kthread usages in the whole
subsystem.
Below are some highlights:
ASoC:
- Support for stereo DAPM controls
- Some initial work on the of-graph sound card
- regmap conversions of the remaining AC'97 drivers
- A new version of the topology ABI; this should be backward compatible
- Updates / cleanups of rsnd, sunxi, sti, nau8825, samsung, arizona,
Intel skylake, atom-sst
- New drivers for Cirrus Logic CS42L42, Qualcomm MSM8916-WCD, and
Realtek RT5665
USB-audio:
- Yet another race fix at disconnection
- Tolerated packet size calculation for some Android devices
- Quirks for Axe-Fx II, QuickCam, TEAC 501/503
HD-audio:
- Improvement of Dell pin fixup mapping
- Quirks for HP Z1 Gen3, Alienware 15 R2 2016 and ALC622 headset mic
Misc:
- Replace all kthread usages with simple works
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAABAgAGBQJYUGNQAAoJEGwxgFQ9KSmkc3kP/jS5EDPWb6dwqdlZomT0lumR
flpvLLo6TWTi9CS8MkC1hPHbR+3LGPMUL8Wwf2cVb06dvKpUHa5P3Wq6lMm6T9wO
1ExhnHeDEDfGXop3/EuADmpRsfsV+z/PpWYmGB0vkoyPTg7e3xQRA6Im6p6IMi45
pnvEaSe18oEidqzAxaBgBFTdoNyaD9zrPL5J9Jh0/JCZ/kSmY6hhI+hlTjjOKCR6
fs4ge2WreonAKbhfOE61eE2KDnQA4II/2JUoPA42wZOat9ukb8DiErgHoFgsWTtE
h+/ZN5Qj3dweP3+NIKsWrmfC6qzTt9OnUNPB2O0A+UYmAMQBjBz9KVxW1FHyTqBp
ewATtvCNIQs+uPMaO/Q/Qo3ZkKLf/W7o/Dt2u/orSrnQ2plKgIOiHJodhXlTrOWm
szF+qJq0xRB7+/dZa+gDjGPAjirr6n3Fv7wrJmqfyF0hLaIEy4wa2Rd/IAZ9Ar46
5S4prL2Ba1CKf1YV27DR1bjUTAonlApfczNp+urCGQJCfZ0kQOAhZJQ/mbMOgSDS
KEC0vY6NLr88b19OETS4oQZ0ixxIAm8hwAle6z39GGhGZ8JWH5+JRzy8emVh2bss
lpFLFHLwK+a+JHxmnOniJW9AlC8I7hVqfS63qaTXfvDnMMcRd5fMgniZt8bTWc2/
F5hsE83JUlcA8Owfyoca
=WZeb
-----END PGP SIGNATURE-----
Merge tag 'sound-4.10-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
Pull sound updates from Takashi Iwai:
"No dramatic changes are found in this development cycle, but as usual,
many commits are applied in a wide range of drivers.
Most of big changes are in ASoC, where a few bits of framework work
and quite a lot of cleanups and improvements to existing code have
been done. The rest are usual stuff, a few HD-audio and USB-audio
quirks and fixes, as well as the drop of kthread usages in the whole
subsystem.
Below are some highlights:
ASoC:
- support for stereo DAPM controls
- some initial work on the of-graph sound card
- regmap conversions of the remaining AC'97 drivers
- a new version of the topology ABI; this should be backward
compatible
- updates / cleanups of rsnd, sunxi, sti, nau8825, samsung, arizona,
Intel skylake, atom-sst
- new drivers for Cirrus Logic CS42L42, Qualcomm MSM8916-WCD, and
Realtek RT5665
USB-audio:
- yet another race fix at disconnection
- tolerated packet size calculation for some Android devices
- quirks for Axe-Fx II, QuickCam, TEAC 501/503
HD-audio:
- improvement of Dell pin fixup mapping
- quirks for HP Z1 Gen3, Alienware 15 R2 2016 and ALC622 headset mic
Misc:
- replace all kthread usages with simple works"
* tag 'sound-4.10-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound: (296 commits)
ALSA: hiface: Fix M2Tech hiFace driver sampling rate change
ALSA: usb-audio: Eliminate noise at the start of DSD playback.
ALSA: usb-audio: Add native DSD support for TEAC 501/503 DAC
ASoC: wm_adsp: wm_adsp_buf_alloc should use kfree in error path
ASoC: topology: avoid uninitialized kcontrol_type
ALSA: usb-audio: Add QuickCam Communicate Deluxe/S7500 to volume_control_quirks
ALSA: usb-audio: add implicit fb quirk for Axe-Fx II
ASoC: zte: spdif: correct ZX_SPDIF_CLK_RAT define
ASoC: zte: spdif and i2s drivers are not zx296702 specific
ASoC: rsnd: setup BRGCKR/BRRA/BRRB when starting
ASoC: rsnd: enable/disable ADG when suspend/resume timing
ASoC: rsnd: tidyup ssi->usrcnt counter check in hw_params
ALSA: cs46xx: add a new line
ASoC: Intel: update bxt_da7219_max98357a to support quad ch dmic capture
ASoC: nau8825: disable sinc filter for high THD of ADC
ALSA: usb-audio: more tolerant packetsize
ALSA: usb-audio: avoid setting of sample rate multiple times on bus
ASoC: cs35l34: Simplify the logic to set CS35L34_MCLK_CTL setting
ALSA: hda - Gate the mic jack on HP Z1 Gen3 AiO
ALSA: hda: when comparing pin configurations, ignore assoc in addition to seq
...
Commit 4bcc595ccd ("printk: reinstate KERN_CONT for printing
continuation lines") allows to define more message headers for a single
message. The motivation is that continuous lines might get mixed.
Therefore it make sense to define the right log level for every piece of
a cont line.
This patch allows to copy only the real message level. We should ignore
KERN_CONT because <filename:line> is added for each message. By other
words, we want to know where each piece of the line comes from.
[pmladek@suse.com: fix a check of the valid message level]
Link: http://lkml.kernel.org/r/20161111183444.GE2145@dhcp128.suse.cz
Link: http://lkml.kernel.org/r/1478695291-12169-5-git-send-email-pmladek@suse.com
Signed-off-by: Petr Mladek <pmladek@suse.com>
Cc: Joe Perches <joe@perches.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Jason Wessel <jason.wessel@windriver.com>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Chris Mason <clm@fb.com>
Cc: Josef Bacik <jbacik@fb.com>
Cc: David Sterba <dsterba@suse.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Currently the ALSA proc handler allows read or write even if the proc
file were write-only or read-only. It's mostly harmless, does thing
but allocating memory and ignores the input/output. But it doesn't
tell user about the invalid use, and it's confusing and inconsistent
in comparison with other proc files.
This patch adds some sanity checks and let the proc handler returning
an -EIO error when the invalid read/write is performed.
Cc: <stable@vger.kernel.org> # v4.2+
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The ALSA proc handler allows currently the write in the unlimited size
until kmalloc() fails. But basically the write is supposed to be only
for small inputs, mostly for one line inputs, and we don't have to
handle too large sizes at all. Since the kmalloc error results in the
kernel warning, it's better to limit the size beforehand.
This patch adds the limit of 16kB, which must be large enough for the
currently existing code.
Cc: stable@vger.kernel.org # v4.2+
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The recent rewrite of the sequencer time accounting using timespec64
in the commit [3915bf294652: ALSA: seq_timer: use monotonic times
internally] introduced a bad regression. Namely, the time reported
back doesn't increase but goes back and forth.
The culprit was obvious: the delta is stored to the result (cur_time =
delta), instead of adding the delta (cur_time += delta)!
Let's fix it.
Fixes: 3915bf2946 ('ALSA: seq_timer: use monotonic times internally')
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=177571
Reported-by: Yves Guillemot <yc.guillemot@wanadoo.fr>
Cc: <stable@vger.kernel.org> # v4.8+
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This commit is a fix for Linux 4.9-rc1.
In former commit, a function call of compatibility layer for ALSA sequencer
core was obsoleted by an alternative. Although, the alternative gets a
pointer to kernel stack due to mis-programming. As a result, ALSA sequencer
core unexpectedly refers over kernel stack.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Fixes: 8ce8eb601c ("ALSA: seq: add an alternative way to handle ioctl requests")
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Fix a missing \n in a pr_debug message and move the \n to the end
of a pr_err message.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
When checking value of request for copy operation, current implementation
compares shifted value to macros, while these macros are already shifted.
As a result, it never performs to copy from/to user space.
This commit fixes the bug.
Fixes: 8ce8eb601c71('ALSA: seq: add an alternative way to handle ioctl requests'
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
When a seq-virmidi driver is initialized, it registers a rawmidi
instance with its callback to create an associated seq kernel client.
Currently it's done throughly in rawmidi's register_mutex context.
Recently it was found that this may lead to a deadlock another rawmidi
device that is being attached with the sequencer is accessed, as both
open with the same register_mutex. This was actually triggered by
syzkaller, as Dmitry Vyukov reported:
======================================================
[ INFO: possible circular locking dependency detected ]
4.8.0-rc1+ #11 Not tainted
-------------------------------------------------------
syz-executor/7154 is trying to acquire lock:
(register_mutex#5){+.+.+.}, at: [<ffffffff84fd6d4b>] snd_rawmidi_kernel_open+0x4b/0x260 sound/core/rawmidi.c:341
but task is already holding lock:
(&grp->list_mutex){++++.+}, at: [<ffffffff850138bb>] check_and_subscribe_port+0x5b/0x5c0 sound/core/seq/seq_ports.c:495
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&grp->list_mutex){++++.+}:
[<ffffffff8147a3a8>] lock_acquire+0x208/0x430 kernel/locking/lockdep.c:3746
[<ffffffff863f6199>] down_read+0x49/0xc0 kernel/locking/rwsem.c:22
[< inline >] deliver_to_subscribers sound/core/seq/seq_clientmgr.c:681
[<ffffffff85005c5e>] snd_seq_deliver_event+0x35e/0x890 sound/core/seq/seq_clientmgr.c:822
[<ffffffff85006e96>] > snd_seq_kernel_client_dispatch+0x126/0x170 sound/core/seq/seq_clientmgr.c:2418
[<ffffffff85012c52>] snd_seq_system_broadcast+0xb2/0xf0 sound/core/seq/seq_system.c:101
[<ffffffff84fff70a>] snd_seq_create_kernel_client+0x24a/0x330 sound/core/seq/seq_clientmgr.c:2297
[< inline >] snd_virmidi_dev_attach_seq sound/core/seq/seq_virmidi.c:383
[<ffffffff8502d29f>] snd_virmidi_dev_register+0x29f/0x750 sound/core/seq/seq_virmidi.c:450
[<ffffffff84fd208c>] snd_rawmidi_dev_register+0x30c/0xd40 sound/core/rawmidi.c:1645
[<ffffffff84f816d3>] __snd_device_register.part.0+0x63/0xc0 sound/core/device.c:164
[< inline >] __snd_device_register sound/core/device.c:162
[<ffffffff84f8235d>] snd_device_register_all+0xad/0x110 sound/core/device.c:212
[<ffffffff84f7546f>] snd_card_register+0xef/0x6c0 sound/core/init.c:749
[<ffffffff85040b7f>] snd_virmidi_probe+0x3ef/0x590 sound/drivers/virmidi.c:123
[<ffffffff833ebf7b>] platform_drv_probe+0x8b/0x170 drivers/base/platform.c:564
......
-> #0 (register_mutex#5){+.+.+.}:
[< inline >] check_prev_add kernel/locking/lockdep.c:1829
[< inline >] check_prevs_add kernel/locking/lockdep.c:1939
[< inline >] validate_chain kernel/locking/lockdep.c:2266
[<ffffffff814791f4>] __lock_acquire+0x4d44/0x4d80 kernel/locking/lockdep.c:3335
[<ffffffff8147a3a8>] lock_acquire+0x208/0x430 kernel/locking/lockdep.c:3746
[< inline >] __mutex_lock_common kernel/locking/mutex.c:521
[<ffffffff863f0ef1>] mutex_lock_nested+0xb1/0xa20 kernel/locking/mutex.c:621
[<ffffffff84fd6d4b>] snd_rawmidi_kernel_open+0x4b/0x260 sound/core/rawmidi.c:341
[<ffffffff8502e7c7>] midisynth_subscribe+0xf7/0x350 sound/core/seq/seq_midi.c:188
[< inline >] subscribe_port sound/core/seq/seq_ports.c:427
[<ffffffff85013cc7>] check_and_subscribe_port+0x467/0x5c0 sound/core/seq/seq_ports.c:510
[<ffffffff85015da9>] snd_seq_port_connect+0x2c9/0x500 sound/core/seq/seq_ports.c:579
[<ffffffff850079b8>] snd_seq_ioctl_subscribe_port+0x1d8/0x2b0 sound/core/seq/seq_clientmgr.c:1480
[<ffffffff84ffe9e4>] snd_seq_do_ioctl+0x184/0x1e0 sound/core/seq/seq_clientmgr.c:2225
[<ffffffff84ffeae8>] snd_seq_kernel_client_ctl+0xa8/0x110 sound/core/seq/seq_clientmgr.c:2440
[<ffffffff85027664>] snd_seq_oss_midi_open+0x3b4/0x610 sound/core/seq/oss/seq_oss_midi.c:375
[<ffffffff85023d67>] snd_seq_oss_synth_setup_midi+0x107/0x4c0 sound/core/seq/oss/seq_oss_synth.c:281
[<ffffffff8501b0a8>] snd_seq_oss_open+0x748/0x8d0 sound/core/seq/oss/seq_oss_init.c:274
[<ffffffff85019d8a>] odev_open+0x6a/0x90 sound/core/seq/oss/seq_oss.c:138
[<ffffffff84f7040f>] soundcore_open+0x30f/0x640 sound/sound_core.c:639
......
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&grp->list_mutex);
lock(register_mutex#5);
lock(&grp->list_mutex);
lock(register_mutex#5);
*** DEADLOCK ***
======================================================
The fix is to simply move the registration parts in
snd_rawmidi_dev_register() to the outside of the register_mutex lock.
The lock is needed only to manage the linked list, and it's not
necessarily to cover the whole initialization process.
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
When a user timer instance is continued without the explicit start
beforehand, the system gets eventually zero-division error like:
divide error: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN
CPU: 1 PID: 27320 Comm: syz-executor Not tainted 4.8.0-rc3-next-20160825+ #8
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: ffff88003c9b2280 task.stack: ffff880027280000
RIP: 0010:[<ffffffff858e1a6c>] [< inline >] ktime_divns include/linux/ktime.h:195
RIP: 0010:[<ffffffff858e1a6c>] [<ffffffff858e1a6c>] snd_hrtimer_callback+0x1bc/0x3c0 sound/core/hrtimer.c:62
Call Trace:
<IRQ>
[< inline >] __run_hrtimer kernel/time/hrtimer.c:1238
[<ffffffff81504335>] __hrtimer_run_queues+0x325/0xe70 kernel/time/hrtimer.c:1302
[<ffffffff81506ceb>] hrtimer_interrupt+0x18b/0x420 kernel/time/hrtimer.c:1336
[<ffffffff8126d8df>] local_apic_timer_interrupt+0x6f/0xe0 arch/x86/kernel/apic/apic.c:933
[<ffffffff86e13056>] smp_apic_timer_interrupt+0x76/0xa0 arch/x86/kernel/apic/apic.c:957
[<ffffffff86e1210c>] apic_timer_interrupt+0x8c/0xa0 arch/x86/entry/entry_64.S:487
<EOI>
.....
Although a similar issue was spotted and a fix patch was merged in
commit [6b760bb2c63a: ALSA: timer: fix division by zero after
SNDRV_TIMER_IOCTL_CONTINUE], it seems covering only a part of
iceberg.
In this patch, we fix the issue a bit more drastically. Basically the
continue of an uninitialized timer is supposed to be a fresh start, so
we do it for user timers. For the direct snd_timer_continue() call,
there is no way to pass the initial tick value, so we kick out for the
uninitialized case.
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
When the stream is in suspended state some applications wait
on "Stream Pipe Error" in response to snd_pcm_avail call to
resume the stream.
In the current implementation snd_pcm_avail() returns zero
when the stream is in suspended state. This causes application
to enter in infinite loop for frames to be available.
"Stream pipe Error" code is getting returned for read/write
call when the stream is in suspended state. Similarly update
snd_pcm_avail to return -ESTRPIPE.
Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
I got this with syzkaller:
==================================================================
BUG: KASAN: null-ptr-deref on address 0000000000000020
Read of size 32 by task syz-executor/22519
CPU: 1 PID: 22519 Comm: syz-executor Not tainted 4.8.0-rc2+ #169
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2
014
0000000000000001 ffff880111a17a00 ffffffff81f9f141 ffff880111a17a90
ffff880111a17c50 ffff880114584a58 ffff880114584a10 ffff880111a17a80
ffffffff8161fe3f ffff880100000000 ffff880118d74a48 ffff880118d74a68
Call Trace:
[<ffffffff81f9f141>] dump_stack+0x83/0xb2
[<ffffffff8161fe3f>] kasan_report_error+0x41f/0x4c0
[<ffffffff8161ff74>] kasan_report+0x34/0x40
[<ffffffff82c84b54>] ? snd_timer_user_read+0x554/0x790
[<ffffffff8161e79e>] check_memory_region+0x13e/0x1a0
[<ffffffff8161e9c1>] kasan_check_read+0x11/0x20
[<ffffffff82c84b54>] snd_timer_user_read+0x554/0x790
[<ffffffff82c84600>] ? snd_timer_user_info_compat.isra.5+0x2b0/0x2b0
[<ffffffff817d0831>] ? proc_fault_inject_write+0x1c1/0x250
[<ffffffff817d0670>] ? next_tgid+0x2a0/0x2a0
[<ffffffff8127c278>] ? do_group_exit+0x108/0x330
[<ffffffff8174653a>] ? fsnotify+0x72a/0xca0
[<ffffffff81674dfe>] __vfs_read+0x10e/0x550
[<ffffffff82c84600>] ? snd_timer_user_info_compat.isra.5+0x2b0/0x2b0
[<ffffffff81674cf0>] ? do_sendfile+0xc50/0xc50
[<ffffffff81745e10>] ? __fsnotify_update_child_dentry_flags+0x60/0x60
[<ffffffff8143fec6>] ? kcov_ioctl+0x56/0x190
[<ffffffff81e5ada2>] ? common_file_perm+0x2e2/0x380
[<ffffffff81746b0e>] ? __fsnotify_parent+0x5e/0x2b0
[<ffffffff81d93536>] ? security_file_permission+0x86/0x1e0
[<ffffffff816728f5>] ? rw_verify_area+0xe5/0x2b0
[<ffffffff81675355>] vfs_read+0x115/0x330
[<ffffffff81676371>] SyS_read+0xd1/0x1a0
[<ffffffff816762a0>] ? vfs_write+0x4b0/0x4b0
[<ffffffff82001c2c>] ? __this_cpu_preempt_check+0x1c/0x20
[<ffffffff8150455a>] ? __context_tracking_exit.part.4+0x3a/0x1e0
[<ffffffff816762a0>] ? vfs_write+0x4b0/0x4b0
[<ffffffff81005524>] do_syscall_64+0x1c4/0x4e0
[<ffffffff810052fc>] ? syscall_return_slowpath+0x16c/0x1d0
[<ffffffff83c3276a>] entry_SYSCALL64_slow_path+0x25/0x25
==================================================================
There are a couple of problems that I can see:
- ioctl(SNDRV_TIMER_IOCTL_SELECT), which potentially sets
tu->queue/tu->tqueue to NULL on memory allocation failure, so read()
would get a NULL pointer dereference like the above splat
- the same ioctl() can free tu->queue/to->tqueue which means read()
could potentially see (and dereference) the freed pointer
We can fix both by taking the ioctl_lock mutex when dereferencing
->queue/->tqueue, since that's always held over all the ioctl() code.
Just looking at the code I find it likely that there are more problems
here such as tu->qhead pointing outside the buffer if the size is
changed concurrently using SNDRV_TIMER_IOCTL_PARAMS.
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Currently, automatic variable of 'union ioctl_arg' type is initialized
by designated initialization. Although, the actual effect is interpretation
of early element of int type and initialization of 'int pversion'.
Therefore the first field corresponding to int type is initialized to zero.
This is against my expectation to initialize whole fields.
This commit uses memset() to initialize the variable, instead of designated
initialization.
Fixes: 04a56dd8ed ('ALSA: seq: change ioctl command operation to get data in kernel space')
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Reuse existing functionality from memdup_user() instead of keeping
duplicate source code.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Acked-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Former commits change existent functions so that they don't handle data in
kernel space. Copying from/to userspace is done outside of the functions,
thus no need to change address limit of running task.
This commit obsoletes get_fs()/set_fs() and applies corresponding changes.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
In previous commit, a new table for functions with data in kernel space
is added to replace current table.
This commit changes existent functions to fit the table. These functions
are added to the new table and removed from the old table.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
ALSA sequencer is designed with two types of clients; application and
kernel. Operations for each ioctl command should handle data in both of
user space and kernel space, while current implementation just allows them
to handle data in user space. Data in kernel space is handled with change
of address limit of running tasks.
This commit adds a new table to map ioctl commands to corresponding
functions. The functions get data in kernel space. Helper functions to
operate kernel and application clients seek entries from the table.
Especially, the helper function for application is responsible for coping
from user space to kernel space or vise versa.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Takashi Iwai