linux-kernel - Re: [PATCH 00/14] ASoC: Constify local snd_sof_dsp

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: <7490bce3-3bd6-4beb-b8be-d47a6b0a30f0@kernel.org>
Date: Mon, 22 Apr 2024 22:47:43 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
 Liam Girdwood <lgirdwood@...il.com>,
 Peter Ujfalusi <peter.ujfalusi@...ux.intel.com>,
 Bard Liao <yung-chuan.liao@...ux.intel.com>,
 Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>,
 Daniel Baluta <daniel.baluta@....com>,
 Kai Vehmanen <kai.vehmanen@...ux.intel.com>, Mark Brown
 <broonie@...nel.org>, Jaroslav Kysela <perex@...ex.cz>,
 Takashi Iwai <tiwai@...e.com>, Shawn Guo <shawnguo@...nel.org>,
 Sascha Hauer <s.hauer@...gutronix.de>,
 Pengutronix Kernel Team <kernel@...gutronix.de>,
 Fabio Estevam <festevam@...il.com>, Matthias Brugger
 <matthias.bgg@...il.com>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: sound-open-firmware@...a-project.org, linux-sound@...r.kernel.org,
 linux-kernel@...r.kernel.org, imx@...ts.linux.dev,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH 00/14] ASoC: Constify local snd_sof_dsp_ops

On 22/04/2024 22:42, Pierre-Louis Bossart wrote:
> 
>> There are multiple reasons and benefits for const, like compiler
>> optimization, code readability (meaning) up to security improvements,
>> e.g. by some GCC plugins or marking rodata as really non-writeable, so
>> closing some ways of exploits. There are many opportunities here, even
>> if they are not yet enabled.
> 
> Possibly, but the SOF core does not know if the structure it uses is
> rodata or not. Using the 'const' identifier would be misleading.

How so? If core does not modify structure, it should take it via ops,
just like 100 other widely known structures (see checkpatch). Why is
this different?

> 
>>> that's a different interpretation to the 'software' view you're
>>> describing. "this structure will not modified by this function" is not
>>> the same thing as "this structure CANNOT be modified".
>>
>> Yes, but can we please discuss specific patchset then? Patches which
>> change pointers to const have one "interpretation". Patches which modify
>> static or global data have another.
> 
> Just look at sound/soc/sof/intel/mtl.c... The core will sometimes use a

That's a driver (or specific implementation), not core.

> constant structure and sometimes not, depending on the PCI ID reported
> by hardware. This was intentional to override common defaults and make
> the differences limited in scope between hardware generations.


> 
> int sof_mtl_ops_init(struct snd_sof_dev *sdev)
> {
> 	struct sof_ipc4_fw_data *ipc4_data;
> 
> 	/* common defaults */
> 	memcpy(&sof_mtl_ops, &sof_hda_common_ops, sizeof(struct
> snd_sof_dsp_ops)); <<<< THE BASELINE IS CONSTANT

Yes, I saw it and such users are not changed. They won't receive any
safety. But all others are getting safer.

I really do not understand what is the problem here. In entire Linux all
of such changes are welcomed with open arms. So what is different here?

> 
>         <<< THE REST ISN'T.
> 
> 	/* shutdown */
> 	sof_mtl_ops.shutdown = hda_dsp_shutdown;


Best regards,
Krzysztof