Am Mittwoch, 3. Juni 2020, 13:09:19 CEST schrieb Dan Carpenter: Hi Dan, > On Wed, Jun 03, 2020 at 10:08:56AM +0200, Stephan Müller wrote: > > The Jitter RNG is unconditionally allocated as a seed source follwoing > > the patch 97f2650e5040. Thus, the instance must always be deallocated. > > > > Reported-by: syzbot+2e635807decef724a1fa@xxxxxxxxxxxxxxxxxxxxxxxxx > > Fixes: 97f2650e5040 ("crypto: drbg - always seeded with SP800-90B ...") > > Signed-off-by: Stephan Mueller <smueller@xxxxxxxxxx> > > --- > > > > crypto/drbg.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/crypto/drbg.c b/crypto/drbg.c > > index 37526eb8c5d5..33d28016da2d 100644 > > --- a/crypto/drbg.c > > +++ b/crypto/drbg.c > > @@ -1631,6 +1631,9 @@ static int drbg_uninstantiate(struct drbg_state > > *drbg)> > > if (drbg->random_ready.func) { > > > > del_random_ready_callback(&drbg->random_ready); > > cancel_work_sync(&drbg->seed_work); > > > > + } > > + > > + if (drbg->jent) { > > > > crypto_free_rng(drbg->jent); > > drbg->jent = NULL; > > > > } > > free_everything functions never work. For example, "drbg->jent" can be > an error pointer at this point. > > crypto/drbg.c > 1577 if (!drbg->core) { > 1578 drbg->core = &drbg_cores[coreref]; > 1579 drbg->pr = pr; > 1580 drbg->seeded = false; > 1581 drbg->reseed_threshold = drbg_max_requests(drbg); > 1582 > 1583 ret = drbg_alloc_state(drbg); > 1584 if (ret) > 1585 goto unlock; > 1586 > 1587 ret = drbg_prepare_hrng(drbg); > 1588 if (ret) > 1589 goto free_everything; > ^^^^^^^^^^^^^^^^^^^^ > If we hit two failures inside drbg_prepare_hrng() then "drbg->jent" can > be an error pointer. > > 1590 > 1591 if (IS_ERR(drbg->jent)) { > 1592 ret = PTR_ERR(drbg->jent); > 1593 drbg->jent = NULL; > 1594 if (fips_enabled || ret != -ENOENT) > 1595 goto free_everything; > 1596 pr_info("DRBG: Continuing without Jitter > RNG\n"); 1597 } > 1598 > 1599 reseed = false; > 1600 } > 1601 > 1602 ret = drbg_seed(drbg, pers, reseed); > 1603 > 1604 if (ret && !reseed) > 1605 goto free_everything; > 1606 > 1607 mutex_unlock(&drbg->drbg_mutex); > 1608 return ret; > 1609 > 1610 unlock: > 1611 mutex_unlock(&drbg->drbg_mutex); > 1612 return ret; > 1613 > 1614 free_everything: > 1615 mutex_unlock(&drbg->drbg_mutex); > 1616 drbg_uninstantiate(drbg); > ^^^^ > Leading to an Oops. Thanks a lot for the hint - a version 2 should come shortly. > > 1617 return ret; > 1618 } > > regards, > dan carpenter Ciao Stephan