On 14.06.2018 20:08, Michael Büsch wrote: > enable_best_rng() is used in hwrng_unregister() to switch away from the > currently active RNG, if that is the one currently being removed. > However enable_best_rng() might fail, if the next RNG's init routine > fails. In that case enable_best_rng() will return an error code and > the currently active RNG will remain active. > After unregistering this might lead to crashes due to use-after-free. > > Fix this by dropping the currently active RNG, if enable_best_rng() > failed. This will result in no RNG to be active, if the next-best > one failed to initialize. > > This problem was introduced by 142a27f0a731ddcf467546960a5585970ca98e21 > > > Reported-by: Wirz <spam@xxxxxxxxxxxxx> > Tested-by: Wirz <spam@xxxxxxxxxxxxx> > Signed-off-by: Michael Büsch <m@xxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx > > --- > > See this discussion for a crash in b43's hwrng caused by this problem: > https://www.spinics.net/lists/linux-wireless/msg173089.html > > > > diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c > index 91bb98c42a1c..aaf9e5afaad4 100644 > --- a/drivers/char/hw_random/core.c > +++ b/drivers/char/hw_random/core.c > @@ -516,11 +516,18 @@ EXPORT_SYMBOL_GPL(hwrng_register); > > void hwrng_unregister(struct hwrng *rng) > { > + int err; > + > mutex_lock(&rng_mutex); > > list_del(&rng->list); > - if (current_rng == rng) > - enable_best_rng(); > + if (current_rng == rng) { > + err = enable_best_rng(); > + if (err) { > + drop_current_rng(); > + cur_rng_set_by_user = 0; > + } > + } > > if (list_empty(&rng_list)) { > mutex_unlock(&rng_mutex); > > > Yes, if the hw_init() of the newly chosen rng fails, enable_best_rng() comes back with an error and did not drop the current rng. The patch handles this case. I did not test it, but the code looks fine. reviewed-by Harald Freudenberger <freude@xxxxxxxxxxxxx>
