iwlwifi: fix kernel crash when unregistering thermal zone

iwlwifi: fix kernel crash when unregistering thermal zone

[Jens Axboe]

A recent firmware change seems to have enabled thermal zones on the
iwlwifi driver. Unfortunately, my device fails when registering the
thermal zone. This doesn't stop the driver from attempting to unregister
the thermal zone at unload time, triggering a NULL pointer deference in
strlen() off the thermal_zone_device_unregister() path.

Don't unregister if name is NULL, for that case we failed registering.
Do the same for the cooling zone.

Signed-off-by: Jens Axboe <axboe@fb.com>

---

Would be great if this could go into the current series, as sometimes I
have to reload the driver. Right now I can't, since it crashes...

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
index 63a051be832e..bec7d9c46087 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
@@ -843,8 +843,10 @@ static void iwl_mvm_thermal_zone_unregister(struct iwl_mvm *mvm)
 		return;
 
 	IWL_DEBUG_TEMP(mvm, "Thermal zone device unregister\n");
-	thermal_zone_device_unregister(mvm->tz_device.tzone);
-	mvm->tz_device.tzone = NULL;
+	if (mvm->tz_device.tzone) {
+		thermal_zone_device_unregister(mvm->tz_device.tzone);
+		mvm->tz_device.tzone = NULL;
+	}
 }
 
 static void iwl_mvm_cooling_device_unregister(struct iwl_mvm *mvm)
@@ -853,8 +855,10 @@ static void iwl_mvm_cooling_device_unregister(struct iwl_mvm *mvm)
 		return;
 
 	IWL_DEBUG_TEMP(mvm, "Cooling device unregister\n");
-	thermal_cooling_device_unregister(mvm->cooling_dev.cdev);
-	mvm->cooling_dev.cdev = NULL;
+	if (mvm->cooling_dev.cdev) {
+		thermal_cooling_device_unregister(mvm->cooling_dev.cdev);
+		mvm->cooling_dev.cdev = NULL;
+	}
 }
 #endif /* CONFIG_THERMAL */

Re: iwlwifi: fix kernel crash when unregistering thermal zone

[Kalle Valo]

Jens Axboe <axboe@kernel.dk> writes:

> A recent firmware change seems to have enabled thermal zones on the
> iwlwifi driver. Unfortunately, my device fails when registering the
> thermal zone. This doesn't stop the driver from attempting to unregister
> the thermal zone at unload time, triggering a NULL pointer deference in
> strlen() off the thermal_zone_device_unregister() path.
>
> Don't unregister if name is NULL, for that case we failed registering.
> Do the same for the cooling zone.
>
> Signed-off-by: Jens Axboe <axboe@fb.com>
>
> ---
>
> Would be great if this could go into the current series, as sometimes I
> have to reload the driver. Right now I can't, since it crashes...

Luca, can I take this directly to wireless-drivers? This is an important
fix and we should get it to Linus' tree ASAP.

-- 
Kalle Valo

Re: iwlwifi: fix kernel crash when unregistering thermal zone

[Kalle Valo]

Jens Axboe <axboe@kernel.dk> wrote:
> A recent firmware change seems to have enabled thermal zones on the
> iwlwifi driver. Unfortunately, my device fails when registering the
> thermal zone. This doesn't stop the driver from attempting to unregister
> the thermal zone at unload time, triggering a NULL pointer deference in
> strlen() off the thermal_zone_device_unregister() path.
> 
> Don't unregister if name is NULL, for that case we failed registering.
> Do the same for the cooling zone.
> 
> Signed-off-by: Jens Axboe <axboe@fb.com>

Patch applied to wireless-drivers.git, thanks.

92549cdc288f iwlwifi: fix kernel crash when unregistering thermal zone

-- 
https://patchwork.kernel.org/patch/9522187/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: iwlwifi: fix kernel crash when unregistering thermal zone

[Jens Axboe]

On 01/21/2017 05:59 AM, Kalle Valo wrote:
> Jens Axboe <axboe@kernel.dk> wrote:
>> A recent firmware change seems to have enabled thermal zones on the
>> iwlwifi driver. Unfortunately, my device fails when registering the
>> thermal zone. This doesn't stop the driver from attempting to unregister
>> the thermal zone at unload time, triggering a NULL pointer deference in
>> strlen() off the thermal_zone_device_unregister() path.
>>
>> Don't unregister if name is NULL, for that case we failed registering.
>> Do the same for the cooling zone.
>>
>> Signed-off-by: Jens Axboe <axboe@fb.com>
> 
> Patch applied to wireless-drivers.git, thanks.
> 
> 92549cdc288f iwlwifi: fix kernel crash when unregistering thermal zone

When is this going out?

-- 
Jens Axboe

Re: iwlwifi: fix kernel crash when unregistering thermal zone

[Kalle Valo]

Jens Axboe <axboe@kernel.dk> writes:

> On 01/21/2017 05:59 AM, Kalle Valo wrote:
>> Jens Axboe <axboe@kernel.dk> wrote:
>>> A recent firmware change seems to have enabled thermal zones on the
>>> iwlwifi driver. Unfortunately, my device fails when registering the
>>> thermal zone. This doesn't stop the driver from attempting to unregister
>>> the thermal zone at unload time, triggering a NULL pointer deference in
>>> strlen() off the thermal_zone_device_unregister() path.
>>>
>>> Don't unregister if name is NULL, for that case we failed registering.
>>> Do the same for the cooling zone.
>>>
>>> Signed-off-by: Jens Axboe <axboe@fb.com>
>> 
>> Patch applied to wireless-drivers.git, thanks.
>> 
>> 92549cdc288f iwlwifi: fix kernel crash when unregistering thermal zone
>
> When is this going out?

Trying to send a pull request to Dave tomorrow.

-- 
Kalle Valo

Re: iwlwifi: fix kernel crash when unregistering thermal zone

[Jens Axboe]

On 01/27/2017 12:54 PM, Kalle Valo wrote:
> Jens Axboe <axboe@kernel.dk> writes:
> 
>> On 01/21/2017 05:59 AM, Kalle Valo wrote:
>>> Jens Axboe <axboe@kernel.dk> wrote:
>>>> A recent firmware change seems to have enabled thermal zones on the
>>>> iwlwifi driver. Unfortunately, my device fails when registering the
>>>> thermal zone. This doesn't stop the driver from attempting to unregister
>>>> the thermal zone at unload time, triggering a NULL pointer deference in
>>>> strlen() off the thermal_zone_device_unregister() path.
>>>>
>>>> Don't unregister if name is NULL, for that case we failed registering.
>>>> Do the same for the cooling zone.
>>>>
>>>> Signed-off-by: Jens Axboe <axboe@fb.com>
>>>
>>> Patch applied to wireless-drivers.git, thanks.
>>>
>>> 92549cdc288f iwlwifi: fix kernel crash when unregistering thermal zone
>>
>> When is this going out?
> 
> Trying to send a pull request to Dave tomorrow.

Great, thanks!

-- 
Jens Axboe

Re: iwlwifi: fix kernel crash when unregistering thermal zone

[Kalle Valo]

"Coelho, Luciano" <luciano.coelho@intel.com> writes:

> On Jan 21, 2017 09:56, Kalle Valo <kvalo@codeaurora.org> wrote:
> Jens Axboe <axboe@kernel.dk> writes: 
>> > A recent firmware change seems to have enabled thermal zones on the 
>> > iwlwifi driver. Unfortunately, my device fails when registering the 
>> > thermal zone. This doesn't stop the driver from attempting to unregister 
>> > the thermal zone at unload time, triggering a NULL pointer deference in 
>> > strlen() off the thermal_zone_device_unregister() path. 
>> > 
>> > Don't unregister if name is NULL, for that case we failed registering. 
>> > Do the same for the cooling zone. 
>> > 
>> > Signed-off-by: Jens Axboe <axboe@fb.com> 
>> > 
>> > --- 
>> > 
>> > Would be great if this could go into the current series, as sometimes I 
>> > have to reload the driver. Right now I can't, since it crashes... 
>> 
>> Luca, can I take this directly to wireless-drivers? This is an important 
>> fix and we should get it to Linus' tree ASAP.
>
> Yes, please go ahead and take it directly.
>
> Acked-by: Luca Coelho <luciano.coelho@intel.com>

Thanks, I'll apply it soon.

-- 
Kalle Valo

Re: iwlwifi: fix kernel crash when unregistering thermal zone

[Coelho, Luciano]

KHJlc2VuZGluZywgc29ycnkgZm9yIHRoZSBIVE1MIHZlcnNpb24gbXkgcGhvbmUgZGVjaWRlZCB0
byB1c2UgZm9yIHRoaXMpDQoNCk9uIEphbiAyMSwgMjAxNyAwOTo1NiwgS2FsbGUgVmFsbyA8a3Zh
bG9AY29kZWF1cm9yYS5vcmc+IHdyb3RlOg0KSmVucyBBeGJvZSA8YXhib2VAa2VybmVsLmRrPiB3
cml0ZXM6IA0KPiA+IEEgcmVjZW50IGZpcm13YXJlIGNoYW5nZSBzZWVtcyB0byBoYXZlIGVuYWJs
ZWQgdGhlcm1hbCB6b25lcyBvbiB0aGUgDQo+ID4gaXdsd2lmaSBkcml2ZXIuIFVuZm9ydHVuYXRl
bHksIG15IGRldmljZSBmYWlscyB3aGVuIHJlZ2lzdGVyaW5nIHRoZSANCj4gPiB0aGVybWFsIHpv
bmUuIFRoaXMgZG9lc24ndCBzdG9wIHRoZSBkcml2ZXIgZnJvbSBhdHRlbXB0aW5nIHRvIHVucmVn
aXN0ZXIgDQo+ID4gdGhlIHRoZXJtYWwgem9uZSBhdCB1bmxvYWQgdGltZSwgdHJpZ2dlcmluZyBh
IE5VTEwgcG9pbnRlciBkZWZlcmVuY2UgaW4gDQo+ID4gc3RybGVuKCkgb2ZmIHRoZSB0aGVybWFs
X3pvbmVfZGV2aWNlX3VucmVnaXN0ZXIoKSBwYXRoLiANCj4gPiANCj4gPiBEb24ndCB1bnJlZ2lz
dGVyIGlmIG5hbWUgaXMgTlVMTCwgZm9yIHRoYXQgY2FzZSB3ZSBmYWlsZWQgcmVnaXN0ZXJpbmcu
IA0KPiA+IERvIHRoZSBzYW1lIGZvciB0aGUgY29vbGluZyB6b25lLiANCj4gPiANCj4gPiBTaWdu
ZWQtb2ZmLWJ5OiBKZW5zIEF4Ym9lIDxheGJvZUBmYi5jb20+IA0KPiA+IA0KPiA+IC0tLSANCj4g
PiANCj4gPiBXb3VsZCBiZSBncmVhdCBpZiB0aGlzIGNvdWxkIGdvIGludG8gdGhlIGN1cnJlbnQg
c2VyaWVzLCBhcyBzb21ldGltZXMgSSANCj4gPiBoYXZlIHRvIHJlbG9hZCB0aGUgZHJpdmVyLiBS
aWdodCBub3cgSSBjYW4ndCwgc2luY2UgaXQgY3Jhc2hlcy4uLiANCj4gDQo+IEx1Y2EsIGNhbiBJ
IHRha2UgdGhpcyBkaXJlY3RseSB0byB3aXJlbGVzcy1kcml2ZXJzPyBUaGlzIGlzIGFuIGltcG9y
dGFudCANCj4gZml4IGFuZCB3ZSBzaG91bGQgZ2V0IGl0IHRvIExpbnVzJyB0cmVlIEFTQVAuDQoN
ClllcywgcGxlYXNlIGdvIGFoZWFkIGFuZCB0YWtlIGl0IGRpcmVjdGx5Lg0KDQpBY2tlZC1ieTog
THVjYSBDb2VsaG8gPGx1Y2lhbm8uY29lbGhvQGludGVsLmNvbT4NCg0KVGhhbmtzIQ0KDQotLQ0K
Q2hlZXJzLA0KTHVjYS4=