linux-kernel - hdmi-codec: modifying params in hw

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-next>] [day] [month] [year] [list]

Message-ID: <20190228121516.swd3vweisgcxlvld@shell.armlinux.org.uk>
Date:   Thu, 28 Feb 2019 12:15:16 +0000
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Jyri Sarha <jsarha@...com>, Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>
Cc:     alsa-devel@...a-project.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: hdmi-codec: modifying params in hw_params() callback

Hi all,

While looking at hdmi-codec issues, I spotted this code:

static int hdmi_codec_hw_params(struct snd_pcm_substream *substream,
                                struct snd_pcm_hw_params *params,
                                struct snd_soc_dai *dai)
{
...
        if (params_width(params) > 24)
                params->msbits = 24;

params->msbits is a parameter that is negotiated and refined by
libasound, and is part of the ALSA constraint system.  The "Writing an
ALSA driver" documentation says about the hw_params callback:

"This is called when the hardware parameter (``hw_params``) is set up
 by the application, that is, once when the buffer size, the period
 size, the format, etc. are defined for the pcm substream."

which suggests we should only be reading the parameters, not writing
to them.

What's more is that the msbits is a parameter that is refined with
userspace, so surely the above should be a declared constraint?

Digging a bit deeper, ASoC passes a private copy of the params to each
codec - a copy is made, then fixups for TDM slots are applied, followed
by any topology fixups by the DAI link (be_hw_params_fixup) before the
codec driver's hw_params() callback is made.  Afterwards, ASoC reads
back the rate, channels and physical (memory) width and stores them
in the codec's DAI structure.  The msbits are not read.

hdmi-codec also seems to do nothing with the msbits parameter other
than the above code.

So, all in all, it seems that the above code limiting msbits is
redundant - nothing will read this modified value.  Can we kill it?

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up