Hi, On Thu, Apr 06, 2023 at 06:28:01AM -0700, Xu Biang wrote: > Smatch Warns: > sound/firewire/tascam/tascam-stream.c:493 snd_tscm_stream_start_duplex() > warn: missing unwind goto? > > The direct return will cause the stream list of "&tscm->domain" unemptied > and the session in "tscm" unfinished if amdtp_domain_start() returns with > an error. > > Fix this by changing the direct return to a goto which will empty the > stream list of "&tscm->domain" and finish the session in "tscm". > > The snd_tscm_stream_start_duplex() function is called in the prepare > callback of PCM. According to "ALSA Kernel API Documentation", the prepare > callback of PCM will be called many times at each setup. So, if the > "&d->streams" list is not emptied, when the prepare callback is called > next time, snd_tscm_stream_start_duplex() will receive -EBUSY from > amdtp_domain_add_stream() that tries to add an existing stream to the > domain. The error handling code after the "error" label will be executed > in this case, and the "&d->streams" list will be emptied. So not emptying > the "&d->streams" list will not cause an issue. But it is more efficient > and readable to empty it on the first error by changing the direct return > to a goto statement. > > The session in "tscm" has been begun before amdtp_domain_start(), so it > needs to be finished when amdtp_domain_start() fails. > > Fixes: c281d46a51e3 ("ALSA: firewire-tascam: support AMDTP domain") > Signed-off-by: Xu Biang <xubiang@xxxxxxxxxxx> > Reviewed-by: Dan Carpenter <error27@xxxxxxxxx> > --- > Note that this finding is from static analysis and not tested. > > sound/firewire/tascam/tascam-stream.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Indeed. I overlooked it when posting the patch. The bug exists Linux kernel v5.4 or later and the fix should be forward to stable kernels. Acked-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx> > diff --git a/sound/firewire/tascam/tascam-stream.c b/sound/firewire/tascam/tascam-stream.c > index 53e094cc411f..dfe783d01d7d 100644 > --- a/sound/firewire/tascam/tascam-stream.c > +++ b/sound/firewire/tascam/tascam-stream.c > @@ -490,7 +490,7 @@ int snd_tscm_stream_start_duplex(struct snd_tscm *tscm, unsigned int rate) > // packet is important for media clock recovery. > err = amdtp_domain_start(&tscm->domain, tx_init_skip_cycles, true, true); > if (err < 0) > - return err; > + goto error; > > if (!amdtp_domain_wait_ready(&tscm->domain, READY_TIMEOUT_MS)) { > err = -ETIMEDOUT; > -- > 2.17.1 Thanks Takashi Sakamoto