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]
Date:   Tue, 25 May 2021 22:38:45 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Phillip Potter <phil@...lpotter.co.uk>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH 55/69] ASoC: rt5645: add error checking to rt5645_probe
 function

On Mon, May 03, 2021 at 01:57:22PM +0200, Greg Kroah-Hartman wrote:
> From: Phillip Potter <phil@...lpotter.co.uk>
> 
> Check for return value from various snd_soc_dapm_* calls, as many of
> them can return errors and this should be handled. Also, reintroduce
> the allocation failure check for rt5645->eq_param as well. Make all

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
maintainers and mailing lists for the subsystem and any driver
specific maintainers on any patches that you are submitting to
the kernel so that they can be reviewed.

> +exit:
> +	/*
> +	 * If there was an error above, everything will be cleaned up by the
> +	 * caller if we return an error here.  This will be done with a later
> +	 * call to rt5645_remove().
> +	 */
> +	return ret;

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
any routes and widgets that are added, but it doesn't do it by
calling remove() and people shouldn't add code in their remove
functions which does so.

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
about applying this untested - it's really random if things check
these errors since they're basically static checks that we're not
smart enough to do at compile time and the core is pretty loud
when they hit.  I occasionally wonder about just removing the
return codes, I think more callers don't have the checks than do
(certainly in the case of _force_enable() where I was surprised
to find any callers that do), but never got round to it.

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ