Hi,
On Mon, Sep 26, 2022 at 03:55:47PM +0200, Takashi Iwai wrote:
Hi,
this is a patch set for simplifying the reference to the current PCM state by having the local copy in runtime instead of relying on runtime->status indirection. This also hardens against the attack by modifying the mmapped status record.
The overall patches looks good to me and I have no objections, while I have some slight opinions to them in a place of sound driver developer.
The first patch does the basic job in the core PCM side,
The main concern is indirect accessing to state field via some pointer hops. I think addition of helper macro at first step eases centre of your work, like:
``` diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 8c48a5bce88c..f6a160cb8135 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -669,6 +669,20 @@ void snd_pcm_stream_unlock_irqrestore(struct snd_pcm_substream *substream, stream <= SNDRV_PCM_STREAM_LAST; \ stream++)
+/** + * snd_pcm_stream_state - Return state in runtime of the PCM substream. + * @substream: substream to check. runtime should be attached. + * + * Return state in runtime of the PCM substream. The substream should exists and + * runtime should be attached to it. + */ +static inline snd_pcm_state_t snd_pcm_stream_state(const snd_pcm_substream *substream) +{ + snd_BUG_ON(!(sub) || !(sub)->runtime); + + return substream->runtime->status->state; +} ```
As we can see, sound driver programmer sometimes checks state of runtime in their code, thus the macro could helps them as well as centre of your change.
and the second patch flips the PCM status mmap to read-only for hardening,
The patch looks good to me as well as the patch for asihpi driver, and should be in your tree independent from the others.
while the remaining patches are for drivers to follow the core change.
The conversions are straightforward. In most places, it's just replacing runtime->status->state with runtime->state.
The usage of mentioned macro can control the concerned access. Now addition of new field, `state` to runtime structure can be done easily.
Takashi
===
Takashi Iwai (11): ALSA: pcm: Avoid reference to status->state ALSA: pcm: Make mmap status read-only ALSA: aloop: Replace runtime->status->state reference to runtime->state ALSA: firewire: Replace runtime->status->state reference to runtime->state ALSA: hda: Replace runtime->status->state reference to runtime->state ALSA: asihpi: Replace runtime->status->state reference to runtime->state ALSA: usb-audio: Replace runtime->status->state reference to runtime->state ALSA: usx2y: Replace runtime->status->state reference to runtime->state ASoC: intel: Replace runtime->status->state reference to runtime->state ASoC: sh: Replace runtime->status->state reference to runtime->state usb: gadget: Replace runtime->status->state reference to runtime->state
drivers/usb/gadget/function/u_uac1_legacy.c | 4 +- include/sound/pcm.h | 20 ++- sound/core/oss/pcm_oss.c | 42 +++---- sound/core/pcm.c | 9 +- sound/core/pcm_compat.c | 4 +- sound/core/pcm_lib.c | 16 +-- sound/core/pcm_native.c | 128 ++++++++++---------- sound/drivers/aloop.c | 4 +- sound/firewire/bebob/bebob_pcm.c | 4 +- sound/firewire/dice/dice-pcm.c | 4 +- sound/firewire/digi00x/digi00x-pcm.c | 4 +- sound/firewire/fireface/ff-pcm.c | 4 +- sound/firewire/fireworks/fireworks_pcm.c | 4 +- sound/firewire/motu/motu-pcm.c | 4 +- sound/firewire/oxfw/oxfw-pcm.c | 8 +- sound/firewire/tascam/tascam-pcm.c | 4 +- sound/hda/hdmi_chmap.c | 2 +- sound/pci/asihpi/asihpi.c | 2 +- sound/soc/intel/skylake/skl-pcm.c | 4 +- sound/soc/sh/rz-ssi.c | 2 +- sound/usb/pcm.c | 4 +- sound/usb/usx2y/usbusx2yaudio.c | 3 +- sound/usb/usx2y/usx2yhwdeppcm.c | 3 +- 23 files changed, 150 insertions(+), 133 deletions(-)
===
Cc: Bard Liao yung-chuan.liao@linux.intel.com Cc: Cezary Rojewski cezary.rojewski@intel.com Cc: Felipe Balbi balbi@kernel.org Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Kai Vehmanen kai.vehmanen@linux.intel.com Cc: Liam Girdwood liam.r.girdwood@linux.intel.com Cc: Mark Brown broonie@kernel.org Cc: Peter Ujfalusi peter.ujfalusi@linux.intel.com Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Ranjani Sridharan ranjani.sridharan@linux.intel.com Cc: Takashi Sakamoto o-takashi@sakamocchi.jp
-- 2.35.3
Regards
Takashi Sakamoto