linux-kernel - Re: [alsa-devel] [PATCH] ALSA: ac97c: Fix an error handling path in 'atmel_ac97c

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: <s5htw0ohv0y.wl-tiwai@suse.de>
Date:   Thu, 31 Aug 2017 12:49:17 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     Mark Brown <broonie@...nel.org>
Cc:     Alexandre Belloni <alexandre.belloni@...e-electrons.com>,
        Julia Lawall <julia.lawall@...6.fr>,
        alsa-devel@...a-project.org, garsilva@...eddedor.com,
        arvind.yadav.cs@...il.com, bhumirks@...il.com,
        andriy.shevchenko@...ux.intel.com, nicolas.ferre@...rochip.com,
        perex@...ex.cz, kernel-janitors@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>
Subject: Re: [alsa-devel] [PATCH] ALSA: ac97c: Fix an error handling path in 'atmel_ac97c_probe()'

On Thu, 31 Aug 2017 12:37:16 +0200,
Mark Brown wrote:
> 
> On Thu, Aug 31, 2017 at 12:23:14PM +0200, Takashi Iwai wrote:
> 
> > Ah, wait, now I see your point.  It was introduced by the very recent
> > patch through Mark's asoc tree (since it was wrongly labeled as "ASoC"
> > while it isn't).  That patch looks indeed fishy.  The change in
> > atmel_ac97c_resume() is also bad.
> 
> The resume check looks fine?  The function appears to do nothing other
> than the clk_prepare_enable().

Well, the patch behaves correctly but the code is ugly:
	
        int ret = clk_prepare_enable(chip->pclk);

        return ret;


> > So, I'd prefer reverting the wrong commit instead, and leave some
> > comment about the uselessness of clk_prepare_enable() return value
> > check.
> 
> I'd rather keep the error checking there, it means that people don't
> need to open the code and verify it when they go scanning for potential
> problems.

OK.


Takashi