linux-kernel - Re: [GIT PULL] sound updates for 5.9

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: <s5hmu324nh5.wl-tiwai@suse.de>
Date:   Mon, 10 Aug 2020 15:52:38 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     Mark Brown <broonie@...nel.org>
Cc:     John Stultz <john.stultz@...aro.org>,
        Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
        Srini Kandagatla <srinivas.kandagatla@...aro.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Amit Pundir <amit.pundir@...aro.org>,
        Vinod Koul <vkoul@...nel.org>
Subject: Re: [GIT PULL] sound updates for 5.9

On Mon, 10 Aug 2020 14:22:54 +0200,
Mark Brown wrote:
> 
> On Sat, Aug 08, 2020 at 10:07:36AM +0200, Takashi Iwai wrote:
> > Takashi Iwai wrote:
> 
> > > Does the patch below fix the bug?  If so, it's rather a bug in the
> > > commit cf6e26c71bfd ("ASoC: soc-component: merge
> > > snd_soc_component_read() and snd_soc_component_read32()").
> 
> > That said, the commit cf6e26c71bfd dropped the capability of returning
> > an error code from snd_soc_component_read() completely, while many
> > code still expect an error gets returned.  The assumption mentioned in
> > the patch (the error can be ignored) looks too naive.
> 
> I did an audit of the users when the series was posted and wasn't able
> to turn up any code doing anything constructive with the return values,
> but then once you're past probe error handling often makes things worse
> if you try.  This is the first one which actually seems to have had an
> impact.
> 
> > Morimoto-san, Mark, could you address it?  IMO, we may still need two
> > variants in the end again: the former snd_soc_component_read32() that
> > returns the value directly and snd_soc_component_read() that returns 0
> > or an error.  Only once after we deal with the error handling in each
> > caller side, we can unify the read functions.
> 
> I'm not sure if that specifically is what we need but yeah we should do
> something, if it fixes things your change is certainly good for the
> immediate problem so could you send it with a signoff please?  

OK, will do soon later.

> With the new code we do now have the core code printing an error message
> if the I/O fails, before they were just being ignored more often than
> not.  This did turn up a couple of cases where drivers were relying on
> being able to do things like silently read from registers that just
> don't exist or aren't currently accessible without any diagnostics which
> is it's own problem :/ (especially the volatile cases).

Yeah, we may need some raw access helper for such a case...


thanks,

Takashi