snd_seq_oss_write() currently returns the raw load_patch() callback
result for SEQ_FULLSIZE events.
That callback is documented as returning 0 on success and -errno on
failure, but snd_seq_oss_write() is the file write path and should
report the number of user bytes consumed on success. Some in-tree
backends also return backend-specific positive values, which can still
be shorter than the original write size.
Return the full byte count for successful SEQ_FULLSIZE writes.
Preserve negative errors and convert any nonnegative completion to the
original count.
Cc: stable@vger.kernel.org
Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
Link: https://patch.msgid.link/20260324-alsa-seq-oss-fullsize-write-return-v1-1-66d448510538@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This was done entirely with mindless brute force, using
git grep -l '\<k[vmz]*alloc_objs*(.*, GFP_KERNEL)' |
xargs sed -i 's/\(alloc_objs*(.*\), GFP_KERNEL)/\1)/'
to convert the new alloc_obj() users that had a simple GFP_KERNEL
argument to just drop that argument.
Note that due to the extreme simplicity of the scripting, any slightly
more complex cases spread over multiple lines would not be triggered:
they definitely exist, but this covers the vast bulk of the cases, and
the resulting diff is also then easier to check automatically.
For the same reason the 'flex' versions will be done as a separate
conversion.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This is the result of running the Coccinelle script from
scripts/coccinelle/api/kmalloc_objs.cocci. The script is designed to
avoid scalar types (which need careful case-by-case checking), and
instead replace kmalloc-family calls that allocate struct or union
object instances:
Single allocations: kmalloc(sizeof(TYPE), ...)
are replaced with: kmalloc_obj(TYPE, ...)
Array allocations: kmalloc_array(COUNT, sizeof(TYPE), ...)
are replaced with: kmalloc_objs(TYPE, COUNT, ...)
Flex array allocations: kmalloc(struct_size(PTR, FAM, COUNT), ...)
are replaced with: kmalloc_flex(*PTR, FAM, COUNT, ...)
(where TYPE may also be *VAR)
The resulting allocations no longer return "void *", instead returning
"TYPE *".
Signed-off-by: Kees Cook <kees@kernel.org>
We used to have a variable declaration with __free() initialized with
NULL. This was to keep the old coding style rule, but recently it's
relaxed and rather recommends to follow the new rule to declare in
place of use for __free() -- which avoids potential deadlocks or UAFs
with nested cleanups.
Although the current code has no bug, per se, let's follow the new
standard and move the declaration to the place of assignment (or
directly assign the allocated result) instead of NULL initializations.
Fixes: 80ccbe91ad ("ALSA: seq: oss/synth: Clean up with guard and auto cleanup")
Fixes: 895a46e034 ("ALSA: seq: oss/midi: Cleanup with guard and auto-cleanup")
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20251216140634.171890-6-tiwai@suse.de
The snd_seq bus got a dedicated probe function. Make use of that. This
fixes a runtime warning about the driver needing to be converted to the
bus probe method.
Note that the remove callback returns void now. The actual return value
was ignored before (see device_remove() in drivers/base/dd.c), so there
is no problem introduced by converting `return -EINVAL` to `return`.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/affb5a7107e9d678ce85dc7f0b87445928cd6b94.1765283601.git.u.kleine-koenig@baylibre.com
Use the auto-cleanup for the refcount management of seq_oss_synth
object. The explicit call of snd_use_lock_free() is dropped by the
magic __free(seq_oss_synth) attribute.
Along with that, replace the manual mutex and spin locks with
guard().
Only code refactoring, and no behavior change.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20250827080520.7544-11-tiwai@suse.de
Use the auto-cleanup for the refcount management of seq_oss_midi
object. The explicit call of snd_use_lock_free() is dropped by the
magic __free(seq_oss_midi) attribute.
Along with that, replace the manual mutex and spin locks with
guard().
Only code refactoring, and no behavior change.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20250827080520.7544-10-tiwai@suse.de
snd_seq_client_ioctl_lock() and *_unlock() are used only from a single
function of the OSS layer, and it's just to wrap the call of
snd_seq_kernel_client_ctl().
Provide another variant of snd_seq_kernel_client_ctl() that takes the
locks internally and drop the ugly snd_seq_client_ioctl_lock() and
*_unlock() implementations, instead.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20250827080520.7544-2-tiwai@suse.de
Use a safer function strscpy() instead of strcpy() for copying to
arrays.
Only idiomatic code replacement, and no functional changes.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20250710100727.22653-4-tiwai@suse.de
The recent bug report spotted on the old OSS sequencer code that tries
to combine incoming SysEx messages to a single sequencer event. This
is good, per se, but it has more demerits:
- The sysex message delivery is delayed until the very last event
- The use of internal buffer forced the serialization
The recent fix in commit 0179488ca9 ("ALSA: seq: oss: Fix races at
processing SysEx messages") addressed the latter, but a better fix is
to handle the sysex messages immediately, i.e. just send each incoming
fragmented sysex message as is. And this patch implements that.
This resulted in a significant cleanup as well.
Note that the only caller of snd_seq_oss_synth_sysex() is
snd_seq_oss_process_event(), and all its callers dispatch the event
immediately, so we can just put the passed buffer pointer to the event
record to be handled.
Reported-and-tested-by: Kun Hu <huk23@m.fudan.edu.cn>
Link: https://lore.kernel.org/2B7E93E4-B13A-4AE4-8E87-306A8EE9BBB7@m.fudan.edu.cn
Link: https://patch.msgid.link/20241231115523.15796-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
OSS sequencer handles the SysEx messages split in 6 bytes packets, and
ALSA sequencer OSS layer tries to combine those. It stores the data
in the internal buffer and this access is racy as of now, which may
lead to the out-of-bounds access.
As a temporary band-aid fix, introduce a mutex for serializing the
process of the SysEx message packets.
Reported-by: Kun Hu <huk23@m.fudan.edu.cn>
Closes: https://lore.kernel.org/2B7E93E4-B13A-4AE4-8E87-306A8EE9BBB7@m.fudan.edu.cn
Cc: <stable@vger.kernel.org>
Link: https://patch.msgid.link/20241230110543.32454-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
*-objs suffix is reserved rather for (user-space) host programs while
usually *-y suffix is used for kernel drivers (although *-objs works
for that purpose for now).
Let's correct the old usages of *-objs in Makefiles.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Link: https://lore.kernel.org/r/20240507135513.14919-2-tiwai@suse.de
There are common patterns where a temporary buffer is allocated and
freed at the exit, and those can be simplified with the recent cleanup
mechanism via __free(kfree).
No functional changes, only code refactoring.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/r/20240222111509.28390-7-tiwai@suse.de
In modern C versions, 'bool' is a keyword that cannot be used as
a variable name, so change this instance use something else, and
change the type to bool instead.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Link: https://lore.kernel.org/r/20240216130211.3828455-1-arnd@kernel.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
ALSA OSS sequencer refers to a global variable max_midi_devs at
creating a new port, storing it to its own field. Meanwhile this
variable may be changed by other sequencer events at
snd_seq_oss_midi_check_exit_port() in parallel, which may cause a data
race.
OTOH, this data race itself is almost harmless, as the access to the
MIDI device is done via get_mdev() and it's protected with a refcount,
hence its presence is guaranteed.
Though, it's sill better to address the data-race from the code sanity
POV, and this patch adds the proper spinlock for the protection.
Reported-by: Abhishek Shah <abhishek.shah@columbia.edu>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/CAEHB2493pZRXs863w58QWnUTtv3HHfg85aYhLn5HJHCwxqtHQg@mail.gmail.com
Link: https://lore.kernel.org/r/20220823072717.1706-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The system port creation in ALSA OSS sequencer was wrongly checked
against to the port number that can be never negative. The error code
should be checked rather against the ioctl call.
This patch corrects the error check.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Link: https://lore.kernel.org/r/20210617134742.6321-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
There are lots of places doing assignments in if condition in ALSA
sequencer core, which is a bad coding style that may confuse readers
and occasionally lead to bugs.
This patch is merely for coding-style fixes, no functional changes.
Link: https://lore.kernel.org/r/20210608140540.17885-57-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
snd_seq_oss_synth_make_info() didn't check the error code from
snd_seq_oss_midi_make_info(), and this leads to the call of strlcpy()
with the uninitialized string as the source, which may lead to the
access over the limit.
Add the proper error check for avoiding the failure.
Reported-by: syzbot+e42504ff21cff05a595f@syzkaller.appspotmail.com
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210115093428.15882-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
strlcpy is deprecated. see: Documentation/process/deprecated.rst
Change the calls that do not use the strlcpy return value to the
preferred strscpy.
Done with cocci script:
@@
expression e1, e2, e3;
@@
- strlcpy(
+ strscpy(
e1, e2, e3);
This cocci script leaves the instances where the return value is
used unchanged.
After this patch, sound/ has 3 uses of strlcpy() that need to be
manually inspected for conversion and changed one day.
$ git grep -w strlcpy sound/
sound/usb/card.c: len = strlcpy(card->longname, s, sizeof(card->longname));
sound/usb/mixer.c: return strlcpy(buf, p->name, buflen);
sound/usb/mixer.c: return strlcpy(buf, p->names[index], buflen);
Miscellenea:
o Remove trailing whitespace in conversion of sound/core/hwdep.c
Link: https://lore.kernel.org/lkml/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Mark Brown <broonie@kernel.org>
Link: https://lore.kernel.org/r/22b393d1790bb268769d0bab7bacf0866dcb0c14.camel@perches.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Recently we applied a fix to cover the whole OSS sequencer ioctls with
the mutex for dealing with the possible races. This works fine in
general, but in theory, this may lead to unexpectedly long stall if an
ioctl like SNDCTL_SEQ_SYNC is issued and an event with the far future
timestamp was queued.
For fixing such a potential stall, this patch changes the mutex lock
applied conditionally excluding such an ioctl command. Also, change
the mutex_lock() with the interruptible version for user to allow
escaping from the big-hammer mutex.
Fixes: 80982c7e83 ("ALSA: seq: oss: Serialize ioctls")
Suggested-by: Pavel Machek <pavel@ucw.cz>
Link: https://lore.kernel.org/r/20200922083856.28572-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Some ioctls via OSS sequencer API may race and lead to UAF when the
port create and delete are performed concurrently, as spotted by a
couple of syzkaller cases. This patch is an attempt to address it by
serializing the ioctls with the existing register_mutex.
Basically OSS sequencer API is an obsoleted interface and was designed
without much consideration of the concurrency. There are very few
applications with it, and the concurrent performance isn't asked,
hence this "big hammer" approach should be good enough.
Reported-by: syzbot+1a54a94bd32716796edd@syzkaller.appspotmail.com
Reported-by: syzbot+9d2abfef257f3e2d4713@syzkaller.appspotmail.com
Suggested-by: Hillf Danton <hdanton@sina.com>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20200804185815.2453-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Apply const prefix to the remaining sequencer code: the static tables
for MIDI macros, RPN/NRPN, and some strings.
Just for minor optimization and no functional changes.
Link: https://lore.kernel.org/r/20200105144823.29547-7-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Here are a collection of small fixes for:
- A race with ASoC HD-audio registration
- LINE6 usb-audio memory overwrite by malformed descriptor
- FireWire MIDI handling
- Missing cast for bit shifts in a few USB-audio quirks
- The wrong function calls in minor OSS sequencer code paths
- A couple of HD-audio quirks
-----BEGIN PGP SIGNATURE-----
iQJCBAABCAAsFiEEIXTw5fNLNI7mMiVaLtJE4w1nLE8FAl0cmzoOHHRpd2FpQHN1
c2UuZGUACgkQLtJE4w1nLE+Tvw/+L6btk7A3kIAFU5RVf1dNVBzuaHcXm0/AZl4I
AfiyVTNtSHSwaxXFSnWuAE1Y6fxVOcs9iRMToxQCZlWqjsF5I8GA8OEpRA2hqrgb
C1BxlNeriOJMoD9wazRM6hmgprmwKS4ssGYEb9EMKyWbKcZ2IAExmQNLKAnd1Uhr
KH8Ryc6CGqYUbtafHCmhpfI7ROgZBDydO7LWWiUpCl2GNFMMeX9Q16WueEjBgNvL
qXWW8r5ix67qZuODKGdhei0JmZGSkrR1uT9EMqx15cks6+NsLAFVRy8/h2k/sbob
ut9VTn4N7kGoXVnqpcOIQNZuvOr8NkG7gDEVILmPqeI+0w/Gm+mhNSSZl3aEo8G2
F/smMeqPSgZQfWVouH56u1QKDps5kCKenSar6a+7SuATLL5LUq4xDkw9o/T/nb9i
8ijp4ZtyKUt+YQvtQrjuGunzbhhaZywTBNodKtxHzqzMPnqGQhma6SWx+bxvLYOO
hoNfU8i2nfg6MoBh1Q6aKZgE+Oz8c2kjOnkQsxV5pYrMGqPMTLNaz+s3+Sq7G9Wc
Zy2lZ1rNJditG/oqyVGfOMt7qu/fOeo//lfX1ZSQ95W+lSs3hYrMeckPpuhWRwCD
YK9zMQ9kuRYooHZJgf0F62EoN+/PUXvozAHXUBsDw7XwWUULX9ffUoZewFUj2NwY
mKer+p8=
=KxxN
-----END PGP SIGNATURE-----
Merge tag 'sound-5.2' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
Pull sound fixes from Takashi Iwai:
"Here are a collection of small fixes for:
- A race with ASoC HD-audio registration
- LINE6 usb-audio memory overwrite by malformed descriptor
- FireWire MIDI handling
- Missing cast for bit shifts in a few USB-audio quirks
- The wrong function calls in minor OSS sequencer code paths
- A couple of HD-audio quirks"
* tag 'sound-5.2' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound:
ALSA: line6: Fix write on zero-sized buffer
ALSA: hda: Fix widget_mutex incomplete protection
ALSA: firewire-lib/fireworks: fix miss detection of received MIDI messages
ALSA: seq: fix incorrect order of dest_client/dest_ports arguments
ALSA: hda/realtek - Change front mic location for Lenovo M710q
ALSA: usb-audio: fix sign unintended sign extension on left shifts
ALSA: hda/realtek: Add quirks for several Clevo notebook barebones
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>
Based on 1 normalized pattern(s):
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation either version 2 of the license or at
your option any later version this program is distributed in the
hope that it will be useful but without any warranty without even
the implied warranty of merchantability or fitness for a particular
purpose see the gnu general public license for more details you
should have received a copy of the gnu general public license along
with this program if not write to the free software foundation inc
59 temple place suite 330 boston ma 02111 1307 usa
extracted by the scancode license scanner the SPDX license identifier
GPL-2.0-or-later
has been chosen to replace the boilerplate/reference in 1334 file(s).
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Allison Randal <allison@lohutok.net>
Reviewed-by: Richard Fontana <rfontana@redhat.com>
Cc: linux-spdx@vger.kernel.org
Link: https://lkml.kernel.org/r/20190527070033.113240726@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Add SPDX license identifiers to all Make/Kconfig files which:
- Have no license information of any form
These files fall under the project license, GPL v2 only. The resulting SPDX
license identifier is:
GPL-2.0-only
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
OSS sequencer emulation still allows to queue and issue the events
that manipulate the client pool concurrently in a racy way. This
patch serializes the access like the normal sequencer write / ioctl
via taking the client ioctl_mutex. Since the access to the sequencer
client is done indirectly via a client id number, a new helper to
take/release the mutex is introduced.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
We have two helpers for queuing a sequencer event from the kernel
client, and both are used only from OSS sequencer layer without any
hop and atomic set. Let's simplify and unify two helpers into one.
No functional change, just a call pattern change.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
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>
As a preparatory patch for the upcoming -Wimplicit-fallthrough
compiler checks, replace with the standard "fall through" annotation.
Unfortunately gcc doesn't understand a chattier text.
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>
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>
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>
As Smatch recently suggested, a few places in OSS sequencer codes may
expand the array directly from the user-space value with speculation,
namely there are a significant amount of references to either
info->ch[] or dp->synths[] array:
sound/core/seq/oss/seq_oss_event.c:315 note_on_event() warn: potential spectre issue 'info->ch' (local cap)
sound/core/seq/oss/seq_oss_event.c:362 note_off_event() warn: potential spectre issue 'info->ch' (local cap)
sound/core/seq/oss/seq_oss_synth.c:470 snd_seq_oss_synth_load_patch() warn: potential spectre issue 'dp->synths' (local cap)
sound/core/seq/oss/seq_oss_event.c:293 note_on_event() warn: potential spectre issue 'dp->synths'
sound/core/seq/oss/seq_oss_event.c:353 note_off_event() warn: potential spectre issue 'dp->synths'
sound/core/seq/oss/seq_oss_synth.c:506 snd_seq_oss_synth_sysex() warn: potential spectre issue 'dp->synths'
sound/core/seq/oss/seq_oss_synth.c:580 snd_seq_oss_synth_ioctl() warn: potential spectre issue 'dp->synths'
Although all these seem doing only the first load without further
reference, we may want to stay in a safer side, so hardening with
array_index_nospec() would still make sense.
We may put array_index_nospec() at each place, but here we take a
different approach:
- For dp->synths[], change the helpers to retrieve seq_oss_synthinfo
pointer directly instead of the array expansion at each place
- For info->ch[], harden in a normal way, as there are only a couple
of places
As a result, the existing helper, snd_seq_oss_synth_is_valid() is
replaced with snd_seq_oss_synth_info(). Also, we cover MIDI device
where a similar array expansion is done, too, although it wasn't
reported by Smatch.
BugLink: https://marc.info/?l=linux-kernel&m=152411496503418&w=2
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
