linux-kernel - Re: [PATCH 3/6] hwrng: imx-rngc

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 17 Feb 2020 10:35:47 +0100
From:   Martin Kaiser <martin@...ser.cx>
To:     PrasannaKumar Muralidharan <prasannatsmkumar@...il.com>
Cc:     Herbert Xu <herbert@...dor.apana.org.au>,
        NXP Linux Team <linux-imx@....com>,
        "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" 
        <linux-crypto@...r.kernel.org>,
        linux-arm-kernel@...ts.infradead.org,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/6] hwrng: imx-rngc - use devres for registration

Hi PrasannaKumar,

Thus wrote PrasannaKumar Muralidharan (prasannatsmkumar@...il.com):

> After imx_rngc_remove function hwrng_unregister will get called. This
> leaves a window where the clock to rng hardware block is disabled but
> still user space can access it via /dev/hwrng.

thanks for spotting this issue. I see that in __device_release_driver,
the driver's remove function is called before the devres cleanup.

> This does not look right, please revisit the patch.

I checked again how other hwrng drivers use devres. Some don't have to
disable a clock and need no remove function at all. Others enable the
clock in the hwrng init routine and disable it in the cleanup routine.

Both of these approaches don't work here. I should disable the clock
eventually and I need it in the probe function to run the selftest
before hwrng init is called.

Therefore, I suggest to drop this patch, at least for the moment.
Herbert, should I resend the series without this patch or is it ok for
you to take the remaining patches as-is?

BTW, 3e75241be808 ("hwrng: drivers - Use device-managed registration
API") makes the same change that I proposed here for a couple of other
hwrng drivers and seems to introduce the same race condition in som
drivers e.g. drivers/char/hw_random/exynos-trng.c. Should we try to fix
this?

Thanks,
Martin