| Message ID | 20220304110124.68988-1-o-takashi@sakamocchi.jp (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ALSA: firewire-lib: fix uninitialized flag for AV/C deferred transaction | expand |
On Fri, 04 Mar 2022 12:01:24 +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> > --- > 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..564607e083ca 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); The "!!" is superfluous as the comparison results are already boolean. thanks, Takashi
diff --git a/sound/firewire/fcp.c b/sound/firewire/fcp.c index bbfbebf4affb..564607e083ca 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);
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(-)