Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Luca Dariz <luca.dariz@hitachi-powergrids.com>
To:     Herbert Xu <herbert@gondor.apana.org.au>
CC:     "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        Matt Mackall <mpm@selenic.com>,
        Colin Ian King <colin.king@canonical.com>,
        Holger Brunck <holger.brunck@hitachi-powergrids.com>,
        Valentin Longchamp <valentin.longchamp@hitachi-powergrids.com>
Subject: RE: [PATCH v2] hwrng: fix khwrng lifecycle
Thread-Topic: [PATCH v2] hwrng: fix khwrng lifecycle
Thread-Index: AQHW05qFV0ZE+WiQXUGvKi1k/RGcnaoU8hoAgERxIBA=
Date:   Mon, 15 Feb 2021 12:52:41 +0000
Message-ID: <AM6PR06MB5400DAFE0551F1D468B728FBAB889@AM6PR06MB5400.eurprd06.prod.outlook.com>
References: <20201216105906.6607-1-luca.dariz@hitachi-powergrids.com>
 <20210102211720.GA1788@gondor.apana.org.au>
In-Reply-To: <20210102211720.GA1788@gondor.apana.org.au>
Accept-Language: en-GB, en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 2f25386e-cb7f-4209-a660-08d8d1b094f1
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Feb 2021 12:52:41.5119
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 7831e6d9-dc6c-4cd1-9ec6-1dc2b4133195
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Fxi7u1Me1sYsu0mG+OuY9X3W1r2tCpvYiOYxre/9D9vxcLpI/mF2a5jNyz86BO/pKw9pJ0aNtEVXpe0Jf9FdrSa00Ts18sxXvt7sT7TYw5NhARgRvbRXXNi3AYYW0NfZ
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR06MB5495
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

>On Wed, Dec 16, 2020 at 11:59:06AM +0100, Luca Dariz wrote:
>>
>> @@ -432,12 +433,15 @@ static int hwrng_fillfn(void *unused)  {
>>       long rc;
>>
>> +     complete(&hwrng_started);
>>       while (!kthread_should_stop()) {
>>               struct hwrng *rng;
>>
>>               rng =3D get_current_rng();
>> -             if (IS_ERR(rng) || !rng)
>> -                     break;
>> +             if (IS_ERR(rng) || !rng) {
>> +                     msleep_interruptible(10);
>> +                     continue;
>
>Please fix this properly with reference counting.

I thought a bit more about it, but I always find a potential race condition=
 with kthread_stop() and the hwrng_fill NULL pointer check.
In my opinion the thread termination should be only triggered with kthread_=
stop(), otherwise it might be called with an invalid or NULL hwrng_fill.
Am I missing something?

Thanks
Luca