2018-06-13 12:01:48

by Michael Büsch

[permalink] [raw]
Subject: Re: b43 crashes on rmmod (bcm4331)

On Wed, 13 Jun 2018 13:09:05 +0200
Michael Büsch <[email protected]> wrote:

> On Wed, 13 Jun 2018 14:01:53 +0300
> Wirz <spam-rxbgZ4vWfLhdz0/[email protected]> wrote:
>
> > > CONFIG_B43_HWRNG completely switches off hwrng support in b43.
> > > I don't see how a change in hwrng core could still lead to a crash in
> > > b43 with this option switched off.
> > >
> > > What do you mean by "manually in the .config"?
> >
> > What I meant is, that I have outcommented the line 'CONFIG_B43_HWRNG=y'
> > in .config.
> >
> > > Can you try to properly switch off the setting with make menuconfig and
> > > then recompile everything from scratch (make clean)?
> >
> > I was intending to do that before, but I cannot find the option for
> > that. Searching for b43_hwrng in menuconfig only shows the dependencies
> > of that option, and the Kconfig file where it is defined, but not the
> > path in menuconfig. Do I indirectly set CONFIG_B43_HWRNG through the
> > parameters it depends on? I'm sorry, but this is obviously above my
> > level of expertise ...
>
> Whoops, sorry. You are right. This is an automatic config option.
> That also means your manual editing of .config would be overridden.
>
> You can edit drivers/net/wireless/broadcom/b43/Kconfig
> go to the section config B43_HWRNG
> and change 'default y' to 'default n'
>
> That should disable it.



Could you please also try the attached patch?
There seems to be a problem in hwrng core in that it does not disable
the current RNG, if the new RNG fails to initialize.
I don't know if that's the problem here, though.


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);



--
Michael


Attachments:
(No filename) (2.16 kB)
rng_new_init_fail.patch (598.00 B)
(No filename) (833.00 B)
OpenPGP digital signature
Download all attachments

2018-06-13 13:07:02

by Wirz

[permalink] [raw]
Subject: Re: b43 crashes on rmmod (bcm4331)


>>>> CONFIG_B43_HWRNG completely switches off hwrng support in b43.
>>>> I don't see how a change in hwrng core could still lead to a crash in
>>>> b43 with this option switched off.
>>>>
>>>> What do you mean by "manually in the .config"?
>>>
>>> What I meant is, that I have outcommented the line 'CONFIG_B43_HWRNG=y'
>>> in .config.
>>>
>>>> Can you try to properly switch off the setting with make menuconfig and
>>>> then recompile everything from scratch (make clean)?
>>>
>>> I was intending to do that before, but I cannot find the option for
>>> that. Searching for b43_hwrng in menuconfig only shows the dependencies
>>> of that option, and the Kconfig file where it is defined, but not the
>>> path in menuconfig. Do I indirectly set CONFIG_B43_HWRNG through the
>>> parameters it depends on? I'm sorry, but this is obviously above my
>>> level of expertise ...
>>
>> Whoops, sorry. You are right. This is an automatic config option.
>> That also means your manual editing of .config would be overridden.
>>
>> You can edit drivers/net/wireless/broadcom/b43/Kconfig
>> go to the section config B43_HWRNG
>> and change 'default y' to 'default n'
>>
>> That should disable it.
>
>
>
> Could you please also try the attached patch?
> There seems to be a problem in hwrng core in that it does not disable
> the current RNG, if the new RNG fails to initialize.
> I don't know if that's the problem here, though.

Ok. Do I apply your patch to the first version that fails for me, and
revert my change to Kconfig?

At the moment the test without B43_HWRNG compiles, I will test that later.


> 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);
>
>
>


--
Do not believe the naysayers who say it cannot be done.


Attachments:
signature.asc (195.00 B)
OpenPGP digital signature