diff options
| author | Cássio Gabriel <cassiogabrielcontato@gmail.com> | 2026-04-17 17:30:18 -0300 |
|---|---|---|
| committer | Takashi Iwai <tiwai@suse.de> | 2026-04-20 17:59:19 +0200 |
| commit | 4cc3ec3d8b3536f2293a5a984c28ba2a09e8b22d (patch) | |
| tree | 116aeda94ea66a0509cc9f2a565a40035b579aeb /sound/pci | |
| parent | 8146cd333d235ed32d48bb803fdf743472d7c783 (diff) | |
ALSA: als4000: Fix capture trigger chip->mode race
snd_als4000_capture_trigger() updates chip->mode under mixer_lock,
while snd_als4000_set_rate() and snd_als4000_playback_trigger()
serialize the same rate-lock state with reg_lock.
The PCM core serializes callbacks only per acted-on substream, or for an
explicitly linked group, so unlinked playback and capture streams can
run concurrently.
That leaves two races on ALS4000 rate-lock state:
- playback and capture trigger callbacks can concurrently update
chip->mode and lose one of the SB_RATE_LOCK bits
- snd_als4000_set_rate() can observe chip->mode without the capture
lock bit set and reprogram the shared sample rate while capture is
being started
Fix this by taking reg_lock as the outer lock in
snd_als4000_capture_trigger() and nesting mixer_lock only for the CR1E
write. This keeps chip->mode serialized with the rest of the ALS4000
rate-lock users while preserving the existing CR1E programming
sequence.
Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
Link: https://patch.msgid.link/20260417-als4000-capture-trigger-race-v1-1-daeffc2feb67@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Diffstat (limited to 'sound/pci')
| -rw-r--r-- | sound/pci/als4000.c | 44 |
1 files changed, 20 insertions, 24 deletions
diff --git a/sound/pci/als4000.c b/sound/pci/als4000.c index 33034e07b3d6..636f309c9424 100644 --- a/sound/pci/als4000.c +++ b/sound/pci/als4000.c @@ -421,30 +421,26 @@ static int snd_als4000_capture_trigger(struct snd_pcm_substream *substream, int { struct snd_sb *chip = snd_pcm_substream_chip(substream); int result = 0; - - /* FIXME race condition in here!!! - chip->mode non-atomic update gets consistently protected - by reg_lock always, _except_ for this place!! - Probably need to take reg_lock as outer (or inner??) lock, too. - (or serialize both lock operations? probably not, though... - racy?) - */ - guard(spinlock)(&chip->mixer_lock); - switch (cmd) { - case SNDRV_PCM_TRIGGER_START: - case SNDRV_PCM_TRIGGER_RESUME: - chip->mode |= SB_RATE_LOCK_CAPTURE; - snd_als4_cr_write(chip, ALS4K_CR1E_FIFO2_CONTROL, - capture_cmd(chip)); - break; - case SNDRV_PCM_TRIGGER_STOP: - case SNDRV_PCM_TRIGGER_SUSPEND: - chip->mode &= ~SB_RATE_LOCK_CAPTURE; - snd_als4_cr_write(chip, ALS4K_CR1E_FIFO2_CONTROL, - capture_cmd(chip)); - break; - default: - result = -EINVAL; - break; + + guard(spinlock)(&chip->reg_lock); + scoped_guard(spinlock, &chip->mixer_lock) { + switch (cmd) { + case SNDRV_PCM_TRIGGER_START: + case SNDRV_PCM_TRIGGER_RESUME: + chip->mode |= SB_RATE_LOCK_CAPTURE; + snd_als4_cr_write(chip, ALS4K_CR1E_FIFO2_CONTROL, + capture_cmd(chip)); + break; + case SNDRV_PCM_TRIGGER_STOP: + case SNDRV_PCM_TRIGGER_SUSPEND: + chip->mode &= ~SB_RATE_LOCK_CAPTURE; + snd_als4_cr_write(chip, ALS4K_CR1E_FIFO2_CONTROL, + capture_cmd(chip)); + break; + default: + result = -EINVAL; + break; + } } return result; } |
