[PATCH v1 0/3] ASoC: Add bounds checking for written values

[Mark Brown]

On Mon, Jan 24, 2022 at 08:20:25PM +0100, Jaroslav Kysela wrote:
> On 24. 01. 22 18:08, Takashi Iwai wrote:

> > Jaroslav, is you concern only about the compatibility of user-space?
> > Or anything else?  The compatibility is always certainly a slight
> > issue; if this breaks really something useful and actually working
> > stuff, we need to consider the workaround...

> My concern is only based on the fact that this code is normally not
> reachable. It only costs some CPU ticks and adds extra code to the drivers
> without any other benefit. The applications should not use out of range
> values and if they do, the behavior is not defined (the drivers should only
> avoid crashes).

> The code seems to be added only to make things consistent for the test
> applications. I don't think that it's worth to care only for this reason.
> What is the goal for this code? The result with the proposed code will be -
> the SoC core has the extra validation conditions.

We need these checks all the time for the generic ASoC controls since
the values for the controls are stored in the underlying device's
register map so the out of range values will be written to the hardware,
pushing it out of specified use.  That's not a great idea in general and
in extreme cases could result in physical damage to the system.  The
biggest risk I see here is around speaker drivers since they deal with
high powers, even ignoring silicon requirements we also don't currently
enforce platform maximums that the machine drivers specify - that
feature was added after an inexperienced user burnt out the speakers in
their Chromebook since the speakers in the system were rated for much
lower powers than the CODEC was able to deliver.

> Basically, I think that those checks should be marked as debug and they
> should be optional like we do for CONFIG_SND_CTL_VALIDATION and enable only
> the fast path by default.

Note also that for everything using these helpers the underlying
register map will be regmap based and with the possible exception of
MMIO based regmaps the cost of writing out the new register value will
be overwhelmingly greater than that of the bounds checks.  It is
extremely hard to envision a scenario in which even a pathological
application would be able to observe a meaningful performance impact.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20220124/e37fd553/attachment.sig>