linux-kernel - Re: [PATCH v4 00/17] ALSA: hda: cirrus: Add initial DSP support and firmware loading

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]

Message-ID: <87wne3wa5w.wl-tiwai@suse.de>
Date:   Mon, 30 May 2022 13:07:55 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     Charles Keepax <ckeepax@...nsource.cirrus.com>
Cc:     Vitaly Rodionov <vitalyr@...nsource.cirrus.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, Mark Brown <broonie@...nel.org>,
        <alsa-devel@...a-project.org>, <patches@...nsource.cirrus.com>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 00/17] ALSA: hda: cirrus: Add initial DSP support and firmware loading

On Mon, 30 May 2022 12:53:29 +0200,
Charles Keepax wrote:
> 
> On Mon, May 30, 2022 at 12:45:08PM +0200, Takashi Iwai wrote:
> > On Mon, 30 May 2022 12:34:15 +0200,
> > Charles Keepax wrote:
> > Well, if an ALSA control can trigger the firmware loading, that's
> > already fragile.  A firmware loading is a heavy task, which should
> > happen only at probing and/or resuming in general.  Do we have other
> > drivers doing the f/w loading triggered by a kctl...?
> > 
> > > I guess we could look at adding locked versions of the add
> > > control functions as well if that might be preferred?
> > 
> > If the patterns of additional kctls (specific for firmware?) are
> > fixed, we may create all such kctls beforehand and let them inactive
> > unless the corresponding firmware is really loaded, too.
> > 
> 
> I am afraid we do, basically all the Wolfson/Cirrus audio devices
> allow you to select the firmware through a kctl. The patterns of
> controls are specific to the firmwares, so we can't really create
> them ahead of time. One could maybe look at changing when the
> firmwares are loaded, such as attempting to load all possible
> firmwares on boot or something but its a fairly sizable change
> that isn't without some side effects.

The call of request_firmawre() itself can be pretty lengthy (e.g. it
may hold until user-space process uploads the firmware if the fallback
mode is enabled), and it implies that the request_firmware() call
doesn't fit well as the operation to be done in a kctl put callback.
So, even if we accept the firmware loading behavior via kctl as-is,
the whole procedure should be async in work instead; namely, not only
kctl creation/deletion but both request_firmware() + post-process
should be done in the work.

And yet moreover, we'll need to consider some way for protecting
against DoS-like behavior by frequent kctl changes.


thanks,

Takashi