linux-kernel - Re: [RESEND PATCH] ALSA: dmaengine_pcm: terminate dmaengine before synchronize

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 24 Jun 2024 14:39:12 +0200
From: Takashi Iwai <tiwai@...e.de>
To: Shengjiu Wang <shengjiu.wang@...il.com>
Cc: Shengjiu Wang <shengjiu.wang@....com>,
	lars@...afoo.de,
	perex@...ex.cz,
	tiwai@...e.com,
	broonie@...nel.org,
	linux-sound@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	alsa-devel@...a-project.org
Subject: Re: [RESEND PATCH] ALSA: dmaengine_pcm: terminate dmaengine before synchronize

On Fri, 21 Jun 2024 04:21:19 +0200,
Shengjiu Wang wrote:
> 
> On Thu, Jun 20, 2024 at 3:56 PM Takashi Iwai <tiwai@...e.de> wrote:
> > But, this may need more rework, too; admittedly it imposes the
> > unnecessary resume of the stream although it shall be stopped and
> > closed immediately after that.  We may have some optimization in
> > addition.
> 
> The suspended_state is not cleared that the resume may be called again
> at the end of stream.

Right, that's the known side effect of this approach.

> Will you push the code?

I'm rethinking of this again, and I'm inclined rather to take your
patch for now.  The side-effect above would be much higher impact in
theory, so I'm not quite sure whether the behavior is acceptable.

Basically, a missing piece is the shortcut state change from SUSPENDED
to CLOSED.  Most drivers don't care as the SUSPENDED state is almost
equivalent with STOPPED state, and they don't support RESUME (hence
the application needs to re-initialize via PREPARE).  But a case like
dmaengine, there can be inconsistency as you pointed out.

By putting snd_pcm_resume() at the beginning of close procedure like
my patch, the state change itself is corrected.  However, it imposes
unnecessary resume, which should be avoided.

Ultimately, we may need some flag or conditional trigger for clearing
this pending SUSPENDED state.  But it needs further consideration.

thanks,

Takashi