Re: b43 crashes on rmmod (bcm4331)

Re: b43 crashes on rmmod (bcm4331)

[Michael Büsch]

[-- Attachment #1.1: Type: text/plain, Size: 2285 bytes --]

On Wed, 13 Jun 2018 13:09:05 +0200
Michael Büsch <m@bues.ch> wrote:

> On Wed, 13 Jun 2018 14:01:53 +0300
> Wirz <spam-rxbgZ4vWfLhdz0/ABlLGQA@public.gmane.org> 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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: rng_new_init_fail.patch --]
[-- Type: text/x-patch, Size: 623 bytes --]

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: b43 crashes on rmmod (bcm4331)

[Wirz]

[-- Attachment #1.1: Type: text/plain, Size: 2411 bytes --]


>>>> 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.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: b43 crashes on rmmod (bcm4331)

[Michael Büsch]

[-- Attachment #1: Type: text/plain, Size: 1941 bytes --]

On Wed, 13 Jun 2018 16:07:02 +0300
Wirz <spam-rxbgZ4vWfLhdz0/ABlLGQA@public.gmane.org> 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.  
> 
> Ok. Do I apply your patch to the first version that fails for me, and
> revert my change to Kconfig?


Yes, please test the patch with a version that would otherwise fail.
You can use 4.16 or the latest kernel for that. I created it with latest
linus' version.


-- 
Michael

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: b43 crashes on rmmod (bcm4331)

[Wirz]

[-- Attachment #1.1: Type: text/plain, Size: 1035 bytes --]


>>>> 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?
> 
> 
> Yes, please test the patch with a version that would otherwise fail.
> You can use 4.16 or the latest kernel for that. I created it with latest
> linus' version.

I tested both suggested cases.  When I disable B43_HWRNG by editing
Kconfig, 'rmmod b43' succeeds in the first version where it previously
failed.  When I apply your patch on top of an unmodified 4.16 it also
succeeds.

cheers, lukas

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: b43 crashes on rmmod (bcm4331)

[Michael Büsch]

[-- Attachment #1: Type: text/plain, Size: 1220 bytes --]

On Thu, 14 Jun 2018 12:47:19 +0300
Wirz <spam-rxbgZ4vWfLhdz0/ABlLGQA@public.gmane.org> wrote:

> >>>> 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?  
> > 
> > 
> > Yes, please test the patch with a version that would otherwise fail.
> > You can use 4.16 or the latest kernel for that. I created it with latest
> > linus' version.  
> 
> I tested both suggested cases.  When I disable B43_HWRNG by editing
> Kconfig, 'rmmod b43' succeeds in the first version where it previously
> failed.  When I apply your patch on top of an unmodified 4.16 it also
> succeeds.


Thank you _very_ much for testing.

I will submit this patch to the hw_random maintainers.

-- 
Michael

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: b43 crashes on rmmod (bcm4331)

[Wirz]

[-- Attachment #1.1: Type: text/plain, Size: 1419 bytes --]

On 14/06/18 19:26, Michael Büsch wrote:
> On Thu, 14 Jun 2018 12:47:19 +0300
> Wirz <spam-rxbgZ4vWfLhdz0/ABlLGQA@public.gmane.org> wrote:
> 
>>>>>> 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?  
>>>
>>>
>>> Yes, please test the patch with a version that would otherwise fail.
>>> You can use 4.16 or the latest kernel for that. I created it with latest
>>> linus' version.  
>>
>> I tested both suggested cases.  When I disable B43_HWRNG by editing
>> Kconfig, 'rmmod b43' succeeds in the first version where it previously
>> failed.  When I apply your patch on top of an unmodified 4.16 it also
>> succeeds.
> 
> 
> Thank you _very_ much for testing.
> 
> I will submit this patch to the hw_random maintainers.

Great, thank you!  I look forward to using a current kernel again.

cheers, lukas


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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]