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

Message-ID: <YLNTmRvIoJWv6lgG@equinox>
Date:   Sun, 30 May 2021 09:58:01 +0100
From:   Phillip Potter <phil@...lpotter.co.uk>
To:     Mark Brown <broonie@...nel.org>
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 Thu, May 27, 2021 at 05:31:57PM +0100, Mark Brown wrote:
> 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.

Dear Mark,

Thank you, I will not resubmit with the new comment in that case. Have a
great weekend.

Regards,
Phil