linux-kernel - Re: [PATCH 55/69] ASoC: rt5645: add error checking to rt5645

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: <YK/JfeMt9j9z+40Q@sirena.org.uk>
Date:   Thu, 27 May 2021 17:31:57 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Phillip Potter <phil@...lpotter.co.uk>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 55/69] ASoC: rt5645: add error checking to rt5645_probe
 function

On Tue, May 25, 2021 at 11:02:16PM +0100, Phillip Potter wrote:
> On Tue, May 25, 2021 at 10:38:45PM +0100, Mark Brown wrote:

> > Phillip, please follow the standard patch submission process,
> > this is documented in submitting-paches.rst in the kernel tree.
> > In particular please make sure that you copy the relevant

> This patch was submitted to a closed mentoring group as part of the
> University of Minnesota reversion/checking process. I was not
> responsible for the final send out to the public mailing lists etc. as
> the patches were collated first and sent out together en masse.

OK, this is really unfortunate.

> > This comment is not accurate, rt5645_remove() just resets the
> > hardware - it's not going to clean up anything to do with any of
> > the branches to error you've got above.  The core *will* clean up

> My comment was adjusted after submission for brevity's sake. This was
> what I originally wrote:
>  /*
>   * All of the above is cleaned up when we return an error here, as
>   * the caller will notice the error and invoke rt5645_remove and
>   * other cleanup routines, as it does for the snd_soc_dapm_* calls
>   * above as well.
>   */
> Happy to resubmit/rewrite as needed? Based on what you've written
> though it may be better to drop the patch?

That is a lot better yes, it accurately reflects what was going
on - the review definitely wasn't helping here.

> > Also I'm guessing this was done purely through inspection rather
> > than the code having been tested?  If there was a problem seen at
> > runtime this isn't fixing it, TBH I'm more than a little dubious

> Yes, that's correct - I did not test this directly other than making
> sure it builds, as I don't have this hardware to test with. 

OK, in that case it's going to be safer to just drop the change,
it's probably not going to cause any actual problems but it's
certainly not something that should go in as a hurried fix.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)