linux-kernel - Re: [PATCH v2] coccinelle: api: semantic patch to use pm_runtime_resume_and

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: <YIgnPkx3NEsOITZH@hovoldconsulting.com>
Date:   Tue, 27 Apr 2021 17:01:18 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Julia Lawall <julia.lawall@...ia.fr>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
        kernel-janitors@...r.kernel.org,
        Gilles Muller <Gilles.Muller@...ia.fr>,
        Nicolas Palix <nicolas.palix@...g.fr>,
        Michal Marek <michal.lkml@...kovi.net>, cocci@...teme.lip6.fr,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] coccinelle: api: semantic patch to use
 pm_runtime_resume_and_get

On Tue, Apr 27, 2021 at 03:44:25PM +0200, Julia Lawall wrote:
> On Tue, 27 Apr 2021, Johan Hovold wrote:
> 
> > On Mon, Apr 26, 2021 at 08:54:04PM +0200, Julia Lawall wrote:
> > > pm_runtime_get_sync keeps a reference count on failure, which can lead
> > > to leaks.  pm_runtime_resume_and_get drops the reference count in the
> > > failure case.  This rule very conservatively follows the definition of
> > > pm_runtime_resume_and_get to address the cases where the reference
> > > count is unlikely to be needed in the failure case.
> > >
> > > pm_runtime_resume_and_get was introduced in
> > > commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to
> > > deal with usage counter")
> > >
> > > Signed-off-by: Julia Lawall <Julia.Lawall@...ia.fr>
> >
> > As I've said elsewhere, not sure trying to do a mass conversion of this
> > is a good idea. People may not be used to the interface, but it is
> > consistent and has its use. The recent flurry of conversions show that
> > those also risk introducing new bugs in code that is currently tested
> > and correct.
> 
> I looked some of the patches you commented on, and this rule would not
> have transformed those cases.  This rule is very restricted to ensure that
> the transformed code follows the behavior of the new function.

Ah, ok. I didn't look too closely at the semantic patch itself and
wrongly associated it with the all-or-nothing media subsystem
conversions.

Thanks for clarifying further in v3 too.

Still a bit worried that this will push the cleanup crew to send more
broken patches since it sends a signal that pm_runtime_get_sync() is
always wrong. But guess there's not much to do about that now after
having added pm_runtime_resume_and_get() in the first place.

Johan