| Age | Commit message (Collapse) | Author |
|---|
|
An HDA virtual-master hook must have its private argument initialized
before a control callback can invoke it. snd_hda_add_vmaster_hook()
installs the trampoline through snd_ctl_add_vmaster_hook(), which stores
master->hook before master->hook_private_data without serialization.
When a codec with a vmaster mute hook binds to an already registered
card, snd_hda_gen_build_controls() publishes and initializes the Master
Playback Switch before installing the hook. A native write that changes
the switch can then run in the following order:
1. The codec bind path publishes and initializes the master control in
__snd_hda_add_vmaster(), then calls snd_hda_add_vmaster_hook().
2. The hook setter stores master->hook, but has not yet stored its
private argument.
3. SNDRV_CTL_IOCTL_ELEM_WRITE takes controls_rwsem and calls
master_put() while the setter remains between those stores.
4. master_put() sees the callback and invokes vmaster_hook() with a
NULL argument. The trampoline dereferences it before the setter
can complete publication.
The write lock held by snd_ctl_elem_write() does not exclude the hook
setter, so this can cause a NULL pointer dereference. Take the same
controls_rwsem for writing inside snd_ctl_add_vmaster_hook() so the
callback and its argument are installed together with respect to
native control callbacks. Pass the card into the helper and update
the HDA caller, keeping the lock in the ALSA core. Release it when
registration returns, before the caller synchronizes the hook.
KASAN report as below:
Oops: general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
CPU: 1 UID: 0 PID: 499 Comm: alsa-vmaster-tr Not tainted 7.2.0-rc6-pmb-bt-functional-v1+ #1 PREEMPT(lazy)
Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
RIP: 0010:vmaster_hook+0x35/0xb0
Code: f5 53 48 89 fb 48 83 ec 08 e8 57 f0 9a fc eb 50 e8 50 f0 9a fc 48 8d 7b 08 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 75 59 48 8d 7b 10 48 8b 43 08 48 ba 00 00 00 00 00 fc
RSP: 0018:ffff888117647cd0 EFLAGS: 00010212
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff84e9ff50
RDX: 0000000000000001 RSI: 0000000000000000 RDI: 0000000000000008
RBP: 0000000000000001 R08: 0000000000000001 R09: 0000000000000001
R10: ffffffff89427457 R11: ffff888115ad1d00 R12: ffffffff84e9ff30
R13: ffff888114943ca0 R14: 0000000000000000 R15: 0000000000000001
FS: 00007fe8167b3780(0000) GS:ffff88826b7e3000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000557c8c2842a8 CR3: 000000011579f005 CR4: 0000000000770ef0
PKRU: 55555554
Call Trace:
<TASK>
master_put+0x20e/0x2b0
snd_ctl_elem_write+0x37f/0x5e0
snd_ctl_ioctl+0x997/0xdd0
? __pfx_snd_ctl_ioctl+0x10/0x10
? srso_alias_return_thunk+0x5/0xfbef5
? selinux_file_ioctl+0xaa/0x280
? __pfx_snd_ctl_ioctl+0x10/0x10
__x64_sys_ioctl+0x163/0x1d0
do_syscall_64+0x115/0x6a0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fe8168c291b
Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1c 48 8b 44 24 18 64 48 2b 04 25 28 00 00
RSP: 002b:00007fffab0cb150 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007fe8168c291b
RDX: 00007fffab0cb310 RSI: 00000000c4c85513 RDI: 0000000000000003
RBP: 00007fffab0cb310 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00007fffab0cb1c0 R14: 0000000000000001 R15: 0000557c4f71fd10
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
Fixes: ee52e56e7b12 ("ALSA: hda - Fix mute-LED fixed mode")
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
Assisted-by: LLM
Link: https://patch.msgid.link/pm-pmb-c2667fb9cc42fbb7943c-v4-b7bc2fe90e950accb3f7@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Currently the places setting or clearing codec->registered flag have
no locking, while its reference in the unsol handling work
snd_hdac_bus_process_unsol_events() takes the bus->reg_lock lock.
Let's add the same bus->reg_lock at the places where codec->registered
is set and cleared for avoiding the races.
Link: https://patch.msgid.link/20261009133341.440917-4-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
When the HD-audio controller driver probe fails, it still leaves the
unsolicited event handling and jackpoll work active, hence if they are
pending, they might fire up later after the resource gets released,
which may lead to a UAF. A similar problem may be seen at shutdown,
too.
Add the recently added helper to disable unsol events and the cancel
of jackpoll_work at the appropriate places. We need the workaround in
the error path before driver->ops->remove, too. As unsol events and
jack polling may have been already enabled during the probe callback,
the driver->ops->remove call is done (more) conditionally now.
Fixes: c3ec8ac82105 ("ASoC: hdac_hda: fix memleak on module unload")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://patch.msgid.link/20261009133341.440917-3-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
The registered field of hdac_device is a bitfield sharing the same
word with other bitfields like lazy_cache, and those are updated in a
non-atomic read-modify-write manner without any lock. Meanwhile, the
registered flag is read from the unsolicited event worker
concurrently, so the concurrent updates of the neighboring bitfields
may race with it.
Convert it to a plain bool for avoiding such a subtle race. Also
replace the assignments of 0/1 with false/true accordingly.
Assisted-by: LLM
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20261007215650.159871-4-tiwai@suse.de
|
|
So far we've used snd_array infrastructure for managing the
hda_jack_tbl elements, but it turned out that this may lead to a UAF
when a different order of destruction procedure is applied; namely,
each hda_jack_tbl object creates a snd_jack object and sets the
private_data pointing to the hda_jack_tbl, and private_free to release
it. It looks harmless, but since snd_array may reallocate the whole
table at growing, the formerly referred jack pointer may become stale.
Fortunately, currently there is no call pattern that triggers this
issue, so it's only hypothetical, but we should address it in anyway.
There are several ways to address this, and at this time, we rather
redesign the hda_jack_tbl allocations: instead of snd_array(), do a
normal kmalloc() + linked list. There will be more kmalloc calls than
the original code, but as the number of jacks are usually at most
handful, it must not be a problem. This change allows us the
persistent jack pointer, so no stale pointer access can happen any
longer.
This also fixes another reported issue about the stale jack pointer
reference in snd_hda_jack_tbl_new(), too.
Fixes: 31ef22579302 ("ALSA: hda - Integrate input-jack stuff into kctl-jack")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20261007171057.7462-1-tiwai@suse.de
|
|
Pull 7.3 devel branch for applying the further HD-audio quirks more
cleanly.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Correct spelling mistakes in comments. No functional change.
Signed-off-by: Hemanth Selam <hemanth.selam@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20260915084424.1007757-3-hemanth.selam@gmail.com
|
|
The put() callback of "IEC958 Playback Default" returns whether the
converted register value moved. The convert_from_spdif_status() helper
reads part of the first two channel status bytes and none of the last
two, while the get() callback returns all four. So a write that lands
only in the bits it does not read changes what userspace reads back and
reports no change. Of the 31 bits above the mode bit, 20 are such bits
in consumer mode and 29 in professional mode. The core notifies only on
a positive return.
Toggling status[2] bit 0 on an HDA HDMI codec moves the read-back from
04 00 00 00 to 04 00 01 00 with no event. Toggling the non-audio bit
in status[0] gives one.
Compare the stored status as well, the way the ac97 code does. The
write to the codec stays gated on the converted value.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
Assisted-by: Claude:claude-opus-5
Link: https://patch.msgid.link/20260908134153.1614273-1-sammiee5311@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Now that the bitwise parameter definitions are gone for PCM and
control parameters, we don't have to cast with ugly __force prefix.
Simply drop those superfluous casts.
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20260812060557.80445-9-tiwai@suse.de
|
|
When userspace writes 1 to /sys/bus/pci/devices/XX/remove to remove
HDA PCI device, the HDA hardware control path is shut down and devres
unmaps the BAR virtual address bus->remap_addr automatically during
driver removal.
If a delayed HDA verb command arrives after the MMIO region is
unmapped, the driver will access invalid virtual addresses and trigger
a page fault splat.
So add an error check right after constructing snd_hda_power_pm.
Signed-off-by: Bob Song <songxiebing@kylinos.cn>
Link: https://patch.msgid.link/20260717024948.506335-1-songxiebing@kylinos.cn
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Currently if an HDMI cable is connected while the system is suspended,
the HDMI audio jack status stays off after resume.
This is due to the jack state not being synced by the HDA HDMI codec
device's runtime resume, as that never happens if the device was runtime
suspended before the system suspended, or by the acomp notification
triggered from the DRM side if that happens before the HDA HDMI codec
device has resumed.
To fix this, if snd_hda_hdmi_acomp_pin_eld_notify() gets called before
the HDA HDMI codec device has resumed, mark it to be forcefully runtime
resumed at the next PM complete time.
Do this using a separate acomp_requested_resume flag that can be
temporarily set without overwriting forced_resume for drivers that
always want to force resume.
Assisted-by: Copilot:claude-sonnet-4.6
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Link: https://patch.msgid.link/20260626-hda-force-resume-eld-notify-v1-1-a92cb01393e0@collabora.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
HD-audio core driver has some open-code for managing the refcount for
PCM instances, and it can be replaced gracefully with the new helpers.
Only a code cleanup, no functional changes.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20260610154538.51076-4-tiwai@suse.de
|
|
snd_hda_ctl_add() dereferences kctl->id.subdevice without checking
whether kctl is NULL. Multiple callers in sound/hda/codecs/ca0132.c
pass the return value of snd_ctl_new1() directly to snd_hda_ctl_add()
without a NULL check:
return snd_hda_ctl_add(codec, nid, snd_ctl_new1(&knew, codec));
snd_ctl_new1() returns NULL when the underlying snd_ctl_new() fails
on memory allocation (kzalloc_flex),which can occur under memory
pressure or via fault injection.
Add a NULL check at the entry of snd_hda_ctl_add(), matching the
pattern already used by snd_ctl_add_replace() at the same call
path (sound/core/control.c:515). Return -EINVAL to let callers
handle the error gracefully.
Fixes: 44f0c9782cc6 ("ALSA: hda/ca0132: Add tuning controls")
Signed-off-by: Quan Sun <2022090917019@std.uestc.edu.cn>
Link: https://patch.msgid.link/20260514132245.3062884-1-2022090917019@std.uestc.edu.cn
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Introduce a common GPIO setup helper function, so that we can clean up
the open code found in many codec drivers later.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20260409093826.1317626-3-tiwai@suse.de
|
|
We used snd_hda_codec_read() for the verb write when a synchronization
is needed after the write, e.g. for the power state toggle or such
cases. It works in principle, but it looks rather confusing and too
hackish.
For improving the code readability, introduce a new helper function,
snd_hda_codec_write_sync(), which is another variant of
snd_hda_codec_write(), and replace the existing snd_hda_codec_read()
calls with this one.
No behavior change but just the code refactoring.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20260409093826.1317626-2-tiwai@suse.de
|
|
The "IEC958 Default PCM Playback Switch" control is backed directly by
mout->share_spdif. The share-switch callbacks currently access that state
without serialization, and spdif_share_sw_put() always returns 0, so
normal userspace writes never emit the standard ALSA control value
notification.
snd_hda_multi_out_analog_open() may also clear mout->share_spdif when the
analog PCM capabilities and the SPDIF capabilities no longer intersect.
That fallback is still needed to avoid creating an impossible hw
constraint set, but it changes the mixer backing value without notifying
subscribers.
Protect the share-switch callbacks with spdif_mutex like the other SPDIF
control handlers, return the actual change value from spdif_share_sw_put(),
and notify the cached control when the open path forcibly disables
shared SPDIF mode after dropping spdif_mutex.
This keeps the existing auto-disable behavior while making switch state
changes visible to userspace.
Fixes: 9a08160bdbe3 ("[ALSA] hda-codec - Add "IEC958 Default PCM" switch")
Fixes: 022b466fc353 ("ALSA: hda - Avoid invalid formats and rates with shared SPDIF")
Suggested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
Link: https://patch.msgid.link/20260403-hda-spdif-share-notify-v3-1-4eb1356b0f17@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
This is the exact same thing as the 'alloc_obj()' version, only much
smaller because there are a lot fewer users of the *alloc_flex()
interface.
As with alloc_obj() version, this was done entirely with mindless brute
force, using the same script, except using 'flex' in the pattern rather
than 'objs*'.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
|
|
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: ee0b0f5d32fe ("ALSA: hda/generic: Use auto cleanup for temporary buffers")
Fixes: 03c5c350e38d ("ALSA: hda/realtek: Add support for new HP G12 laptops")
Fixes: b0550d4c2dd8 ("ALSA: hda/common: Use auto cleanup for temporary buffers")
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20251216140634.171890-10-tiwai@suse.de
|
|
Replace the manual spin lock/unlock pairs with guard() for code
simplification.
Only code refactoring, and no behavior change.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20250827072916.31933-25-tiwai@suse.de
|
|
The release of temporary kmalloced buffers can be nicely handled via
the standard __free(kfree).
Only code refactoring, and no behavior change.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20250827072916.31933-20-tiwai@suse.de
|
|
Replace the manual mutex lock/unlock pairs with guard().
Only code refactoring, and no behavior change.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20250827072916.31933-8-tiwai@suse.de
|
|
The new macro CLASS(snd_hda_power_pm) can replace the manual
snd_hda_power_up_pm() and _down() calls gracefully.
A part of the code in codec_exec_verb() is factored out to a function,
so that the auto-cleanup can be well scoped.
Merely cleanups and no functional changes.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20250827072916.31933-6-tiwai@suse.de
|
|
Use min() to simplify snd_hda_get_devices() and improve its readability.
Change the function parameter 'max_devices' from 'int' to 'unsigned int'
to avoid a min() signedness error. Update all related local variables
and the function's return type to 'unsigned int' accordingly.
No functional changes intended.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Link: https://patch.msgid.link/20250813205507.215658-2-thorsten.blum@linux.dev
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Use 'int' instead of 'unsigned int' because the local variable 'parm'
can be negative.
While an unsigned integer is harmless in practice due to the implicit
type conversion, it's safer and more idiomatic to use a signed integer
to properly check for -1.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Link: https://patch.msgid.link/20250813103418.164110-2-thorsten.blum@linux.dev
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
Use a safer function strscpy() instead of strcpy() for copying to
arrays.
Only idiomatic code replacement, and no functional changes.
Link: https://patch.msgid.link/20250711083051.18759-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
|
snd_hda_codec_build_controls()
The error from snd_hda_codec_init() was ignored in
snd_hda_codec_build_controls(), which should have been taken account
and abort the flow. Fix it now.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20250709160434.1859-28-tiwai@suse.de
|
|
After all conversions, driver->ops became a must in most places
(except for the codec power setup which might be called before binding
to the codec driver), hence we can get rid of the superfluous
driver->ops NULL checks, too.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20250709160434.1859-26-tiwai@suse.de
|
|
Now that all patch_ops usage have been converted to the new
hda_codec_ops probe, we can drop patch_ops from the hda_codec,
together with the calls of patch_ops callbacks.
The hda_codec_ops.free callback is removed as all have been replaced
with the new remove callback.
Also, correct comments mentioning "patch"; it's replaced with "codec
driver".
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20250709160434.1859-25-tiwai@suse.de
|
|
Until now, we use "patch_ops" embedded in hda_codec object for
defining the callbacks that are used in various places to manage
HD-audio codec. But from the device driver POV, this should have been
rather the driver ops, instead of the callbacks in the codec object.
This patch defines the driver ops for HD-audio codec driver as the
replacement. We reuse the same struct hda_codec_ops, and this is put
as hda_codec_driver.ops. When the driver->ops callbacks are defined,
they are called primarily instead of codec->patch_ops callbacks.
With converting to the driver ops, there is no need to pass the ugly
patch_ops handling in hda_device_id tables. That is, driver_data
field of hda_device_id becomes really optional and it can be used for
passing the codec-specific data (e.g. specifying a model).
The codec entries after the conversion should be with HDA_CODEC_ID()
and co, instead of the former HDA_CODEC_ENTRY().
Once after converting all codec drivers to use driver ops, we can get
rid of codec patch_ops.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20250709160434.1859-10-tiwai@suse.de
|
|
The snd-hda-codec module contains the most of common code used by both
HD-audio controller and codec drivers, and it's basically independent
from PCI. Let's move the code to sound/hda/common directory as a part
of code reorganization.
The hda_ prefix is dropped from the most of file names as it's rather
superfluous.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20250709160434.1859-4-tiwai@suse.de
|
