[PATCH v2] ALSA: firewire-lib: fix uninitialized flag for AV/C deferred transaction

[PATCH v2] ALSA: firewire-lib: fix uninitialized flag for AV/C deferred transaction

[Takashi Sakamoto]

AV/C deferred transaction was supported at a commit 00a7bb81c20f ("ALSA:
firewire-lib: Add support for deferred transaction") while 'deferrable'
flag can be uninitialized for non-control/notify AV/C transactions.
UBSAN reports it:

kernel: ================================================================================
kernel: UBSAN: invalid-load in /build/linux-aa0B4d/linux-5.15.0/sound/firewire/fcp.c:363:9
kernel: load of value 158 is not a valid value for type '_Bool'
kernel: CPU: 3 PID: 182227 Comm: irq/35-firewire Tainted: P           OE     5.15.0-18-generic #18-Ubuntu
kernel: Hardware name: Gigabyte Technology Co., Ltd. AX370-Gaming 5/AX370-Gaming 5, BIOS F42b 08/01/2019
kernel: Call Trace:
kernel:  <IRQ>
kernel:  show_stack+0x52/0x58
kernel:  dump_stack_lvl+0x4a/0x5f
kernel:  dump_stack+0x10/0x12
kernel:  ubsan_epilogue+0x9/0x45
kernel:  __ubsan_handle_load_invalid_value.cold+0x44/0x49
kernel:  fcp_response.part.0.cold+0x1a/0x2b [snd_firewire_lib]
kernel:  fcp_response+0x28/0x30 [snd_firewire_lib]
kernel:  fw_core_handle_request+0x230/0x3d0 [firewire_core]
kernel:  handle_ar_packet+0x1d9/0x200 [firewire_ohci]
kernel:  ? handle_ar_packet+0x1d9/0x200 [firewire_ohci]
kernel:  ? transmit_complete_callback+0x9f/0x120 [firewire_core]
kernel:  ar_context_tasklet+0xa8/0x2e0 [firewire_ohci]
kernel:  tasklet_action_common.constprop.0+0xea/0xf0
kernel:  tasklet_action+0x22/0x30
kernel:  __do_softirq+0xd9/0x2e3
kernel:  ? irq_finalize_oneshot.part.0+0xf0/0xf0
kernel:  do_softirq+0x75/0xa0
kernel:  </IRQ>
kernel:  <TASK>
kernel:  __local_bh_enable_ip+0x50/0x60
kernel:  irq_forced_thread_fn+0x7e/0x90
kernel:  irq_thread+0xba/0x190
kernel:  ? irq_thread_fn+0x60/0x60
kernel:  kthread+0x11e/0x140
kernel:  ? irq_thread_check_affinity+0xf0/0xf0
kernel:  ? set_kthread_struct+0x50/0x50
kernel:  ret_from_fork+0x22/0x30
kernel:  </TASK>
kernel: ================================================================================

This commit fixes the bug. The bug has no disadvantage for the non-
control/notify AV/C transactions since the flag has an effect for AV/C
response with INTERIM (0x0f) status which is not used for the transactions
in AV/C general specification.

Fixes: 00a7bb81c20f ("ALSA: firewire-lib: Add support for deferred transaction")
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/fcp.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/sound/firewire/fcp.c b/sound/firewire/fcp.c
index bbfbebf4affb..df44dd5dc4b2 100644
--- a/sound/firewire/fcp.c
+++ b/sound/firewire/fcp.c
@@ -240,9 +240,7 @@ int fcp_avc_transaction(struct fw_unit *unit,
 	t.response_match_bytes = response_match_bytes;
 	t.state = STATE_PENDING;
 	init_waitqueue_head(&t.wait);
-
-	if (*(const u8 *)command == 0x00 || *(const u8 *)command == 0x03)
-		t.deferrable = true;
+	t.deferrable = (*(const u8 *)command == 0x00 || *(const u8 *)command == 0x03);
 
 	spin_lock_irq(&transactions_lock);
 	list_add_tail(&t.list, &transactions);
-- 
2.34.1

Re: [PATCH v2] ALSA: firewire-lib: fix uninitialized flag for AV/C deferred transaction

[Takashi Iwai]

On Fri, 04 Mar 2022 13:56:47 +0100,
Takashi Sakamoto wrote:
> 
> AV/C deferred transaction was supported at a commit 00a7bb81c20f ("ALSA:
> firewire-lib: Add support for deferred transaction") while 'deferrable'
> flag can be uninitialized for non-control/notify AV/C transactions.
> UBSAN reports it:
> 
> kernel: ================================================================================
> kernel: UBSAN: invalid-load in /build/linux-aa0B4d/linux-5.15.0/sound/firewire/fcp.c:363:9
> kernel: load of value 158 is not a valid value for type '_Bool'
> kernel: CPU: 3 PID: 182227 Comm: irq/35-firewire Tainted: P           OE     5.15.0-18-generic #18-Ubuntu
> kernel: Hardware name: Gigabyte Technology Co., Ltd. AX370-Gaming 5/AX370-Gaming 5, BIOS F42b 08/01/2019
> kernel: Call Trace:
> kernel:  <IRQ>
> kernel:  show_stack+0x52/0x58
> kernel:  dump_stack_lvl+0x4a/0x5f
> kernel:  dump_stack+0x10/0x12
> kernel:  ubsan_epilogue+0x9/0x45
> kernel:  __ubsan_handle_load_invalid_value.cold+0x44/0x49
> kernel:  fcp_response.part.0.cold+0x1a/0x2b [snd_firewire_lib]
> kernel:  fcp_response+0x28/0x30 [snd_firewire_lib]
> kernel:  fw_core_handle_request+0x230/0x3d0 [firewire_core]
> kernel:  handle_ar_packet+0x1d9/0x200 [firewire_ohci]
> kernel:  ? handle_ar_packet+0x1d9/0x200 [firewire_ohci]
> kernel:  ? transmit_complete_callback+0x9f/0x120 [firewire_core]
> kernel:  ar_context_tasklet+0xa8/0x2e0 [firewire_ohci]
> kernel:  tasklet_action_common.constprop.0+0xea/0xf0
> kernel:  tasklet_action+0x22/0x30
> kernel:  __do_softirq+0xd9/0x2e3
> kernel:  ? irq_finalize_oneshot.part.0+0xf0/0xf0
> kernel:  do_softirq+0x75/0xa0
> kernel:  </IRQ>
> kernel:  <TASK>
> kernel:  __local_bh_enable_ip+0x50/0x60
> kernel:  irq_forced_thread_fn+0x7e/0x90
> kernel:  irq_thread+0xba/0x190
> kernel:  ? irq_thread_fn+0x60/0x60
> kernel:  kthread+0x11e/0x140
> kernel:  ? irq_thread_check_affinity+0xf0/0xf0
> kernel:  ? set_kthread_struct+0x50/0x50
> kernel:  ret_from_fork+0x22/0x30
> kernel:  </TASK>
> kernel: ================================================================================
> 
> This commit fixes the bug. The bug has no disadvantage for the non-
> control/notify AV/C transactions since the flag has an effect for AV/C
> response with INTERIM (0x0f) status which is not used for the transactions
> in AV/C general specification.
> 
> Fixes: 00a7bb81c20f ("ALSA: firewire-lib: Add support for deferred transaction")
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>

Thanks, applied.


Takashi