linux-kernel - Re: [SIL2review] [PATCH] i2c: img-scb: fix PM device usage count

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: <20180225172730.2b443978@denkmatte>
Date:   Sun, 25 Feb 2018 17:27:30 +0100
From:   Tobias Jordan <Tobias.Jordan@...ktrobit.com>
To:     Nicholas Mc Guire <der.herr@...r.at>
Cc:     <linux-i2c@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Wolfram Sang <wsa@...-dreams.de>
Subject: Re: [SIL2review] [PATCH] i2c: img-scb: fix PM device usage count

Hi!

On Sun, 25 Feb 2018 13:20:14 +0000 Nicholas Mc Guire wrote:

> the decrement though is conditional:
>   pm_runtime_put_noidle
>   -> atomic_add_unless(&dev->power.usage_count, -1, 0);  

pm_runtime_put_noidle is playing it safe by not decrementing past 0, I
think that's a good thing.

> Also just wondering - could one not decrement in pm_runtime_get_sync
> on the error path rather than defering this to the caller and fixing
> it there ?

That's what I've asked linux-pm in the linked discussion:

> > https://marc.info/?l=linux-pm&m=151904483924999&w=2

As far as I've understood the idea is that most "error" return values
actually are a result of disabled runtime PM, and that should be
transparent to the caller. Looking at the code, that's what the vast
majority of callers do - they just ignore the return value of
pm_runtime_get_sync, and somewhere later have an
unconditional pm_runtime_put_... call.

So the only issue are callers that don't ignore the pm_runtime_get_sync
return value, probably because they're having some kind of special
requirements for error handling. For those, they need to ensure that a
proper _put_ is done somewhere in the error path.

> Reviewed-by: Nicholas Mc Guire <der.herr@...r.at>

Thanks for the review!,

Tobias