[PATCH 1/2] ALSA: hdsp: Serialize control_register updates in MIDI input paths

[Yifei Gao]

hdsp->control_register is a software copy of a hardware register. It is
updated read-modify-write under hdsp->lock, except in two MIDI paths:
snd_hdsp_midi_input_read() holds only the per-port hmidi->lock, and
snd_hdsp_interrupt() holds no lock at all. Lock holders disable
interrupts only on the local CPU, so updates can be lost across CPUs.
A lost MIDI interrupt enable bit stops MIDI input on that port. Other
lost updates revert control or PCM settings.

Take hdsp->lock in both places. In snd_hdsp_midi_input_read(), drop
hmidi->lock before taking it, as commit c0da00145f9a ("ALSA: hdspm -
Fix locking in snd_hdspm_midi_input_read") did for hdspm.

Found by static analysis. Compile-tested only; no hardware available.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Yifei Gao <gyf161023@xxxxxxxxx>
---
 sound/pci/rme9652/hdsp.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
index 7dce548b9f36..4e9f3a95fe0d 100644
--- a/sound/pci/rme9652/hdsp.c
+++ b/sound/pci/rme9652/hdsp.c
@@ -1347,6 +1347,10 @@ static int snd_hdsp_midi_input_read (struct hdsp_midi *hmidi)
 			}
 		}
 		hmidi->pending = 0;
+	}
+
+	/* control_register is shared by all users, serialize on hdsp->lock */
+	scoped_guard(spinlock_irqsave, &hmidi->hdsp->lock) {
 		if (hmidi->id)
 			hmidi->hdsp->control_register |= HDSP_Midi1InterruptEnable;
 		else
@@ -3869,8 +3873,10 @@ static irqreturn_t snd_hdsp_interrupt(int irq, void *dev_id)
 	if (midi0 && midi0status) {
 		if (hdsp->use_midi_work) {
 			/* we disable interrupts for this input until processing is done */
-			hdsp->control_register &= ~HDSP_Midi0InterruptEnable;
-			hdsp_write(hdsp, HDSP_controlRegister, hdsp->control_register);
+			scoped_guard(spinlock, &hdsp->lock) {
+				hdsp->control_register &= ~HDSP_Midi0InterruptEnable;
+				hdsp_write(hdsp, HDSP_controlRegister, hdsp->control_register);
+			}
 			hdsp->midi[0].pending = 1;
 			schedule = 1;
 		} else {
@@ -3880,8 +3886,10 @@ static irqreturn_t snd_hdsp_interrupt(int irq, void *dev_id)
 	if (hdsp->io_type != Multiface && hdsp->io_type != RPM && hdsp->io_type != H9632 && midi1 && midi1status) {
 		if (hdsp->use_midi_work) {
 			/* we disable interrupts for this input until processing is done */
-			hdsp->control_register &= ~HDSP_Midi1InterruptEnable;
-			hdsp_write(hdsp, HDSP_controlRegister, hdsp->control_register);
+			scoped_guard(spinlock, &hdsp->lock) {
+				hdsp->control_register &= ~HDSP_Midi1InterruptEnable;
+				hdsp_write(hdsp, HDSP_controlRegister, hdsp->control_register);
+			}
 			hdsp->midi[1].pending = 1;
 			schedule = 1;
 		} else {
-- 
2.34.1