linux-kernel - Re: [PATCH 1/3] crypto: hw_random

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: <20170324161934.kc6g36nazr3y32kp@kozik-lap>
Date:   Fri, 24 Mar 2017 19:19:34 +0300
From:   Krzysztof Kozlowski <krzk@...nel.org>
To:     Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
Cc:     linux-arm-kernel@...ts.infradead.org,
        Kukjin Kim <kgene@...nel.org>,
        Javier Martinez Canillas <javier@....samsung.com>,
        Matt Mackall <mpm@...enic.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>,
        linux-kernel@...r.kernel.org, linux-samsung-soc@...r.kernel.org,
        linux-crypto@...r.kernel.org, Olof Johansson <olof@...om.net>,
        Arnd Bergmann <arnd@...db.de>,
        Marek Szyprowski <m.szyprowski@...sung.com>
Subject: Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver

On Fri, Mar 24, 2017 at 05:11:25PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Friday, March 24, 2017 06:46:00 PM Krzysztof Kozlowski wrote:
> > I really do not like global or file-scope variables. I do not like
> > drivers using them. Actually I hate them.
> > 
> > From time to time I encounter a driver which was designed with that
> > approach - static fields and hidden assumption that there will be only
> > one instance. Usually that assumption is really hidden...
> > 
> > ... and then it happens that I want to use two instances which of course
> > fails.
> > 
> > This code serves as a clear documentation for this assumption - only one
> > instance is allowed. You can look at it as a self-documenting
> > requirement.
> 
> For me it looks as needless case of defensive programming and when
> I see the code like this it always raises questions about the real
> intentions of the code. I find it puzzling and not helpful.

I do not understand what might be puzzling about check for static
file-scope value. It is of course subjective, but for me that looks
pretty self-explanatory.

> 
> > And I think the probe might be called twice, for example in case of
> > mistake in DTB.
> 
> Even if this is possible resource allocation code in the driver will
> take take care of handling it just fine,

Indeed, the devm_ioremap_resource() solves the case. I can drop the
check then.

Best regards,
Krzysztof