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: <9d1eda85-32a0-4e53-86ca-ce3137439bd7@kernel.org>
Date: Mon, 22 Apr 2024 07:43:54 +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 15/04/2024 16:19, Pierre-Louis Bossart wrote:
> 
>> The core code does not modify the 'struct snd_sof_dsp_ops' passed via
>> pointer in various places, so this can be made pointer to const.
> 
> The structure itself is NOT always const - the initialization itself
> does have platform-specific changes, so what do we really gain from all
> this?

In the context of these patches, the structure is *always* const. In
other drivers, it is not, but they are not really relevant here.

> 
> some commit messages say the code is "a bit safer", but I personally
> find the 'const' more confusing since the information that the structure
> can be modified during initialization is lost.

Functions which take some data and do not modify it are easier to read
if the pointed data is marked as const. Then it is obvious that
functions for example is re-entrant. Or that it does not affect the
state of other devices/core structures.

Additionally, the static data is safer when is const, because it cannot
be used in some attacks.

I really do not understand which information you lost here? Core does
not change the ops, so the data should be passed as const as often as
possible. If anyone wants to write a driver which does not use static
ops, but somehow dynamically allocated and changed, nothing stops him.
This patch did not make it less readable/doable.

The point is that these ops do not differ from other ops or some other
driver-passed structures, which we have around 100 already in checkpatch.

Best regards,
Krzysztof