Skip to contentSkip to conversation

ath11k: fix uninitialized return in ath11k_spectral_process_data()

PATCH In review Dan Carpenter 2 participants 7 messages Jun 19, 2020 – Sep 9, 2020

Subsystems (2)
Kernel janitors Qualcomm Atheros ATH11K wireless driver
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 ath11k: fix uninitialized return in ath11k_spectral_process_data()

There is a success path where "ret" isn't initialized where we never have a ATH11K_SPECTRAL_TAG_SCAN_SEARCH and then ret isn't initialized.

Fixes 9d11b7bff950 ("ath11k: add support for spectral scan")
Signed-off-by Dan Carpenter <[email protected]>
---
 drivers/net/wireless/ath/ath11k/spectral.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/ath/ath11k/spectral.c +1 -1
index 1c5d65bb411f..bfbf905f7507 100644
@@ -677,7 +677,7 @@ static int ath11k_spectral_process_data(struct ath11k *ar,
 	u32 data_len, i;
 	u8 sign, tag;
 	int tlv_len, sample_sz;
-	int ret;
+	int ret = 0;
 	bool quit = false;
 
 	spin_lock_bh(&ar->spectral.lock);
Signature · 1 line
2.27.0
#1 lore raw
Quoted 14 lines · Dan Carpenter

There is a success path where "ret" isn't initialized where we never have a ATH11K_SPECTRAL_TAG_SCAN_SEARCH and then ret isn't initialized.

Fixes 9d11b7bff950 ("ath11k: add support for spectral scan")
Signed-off-by Dan Carpenter <[email protected]>
---
 drivers/net/wireless/ath/ath11k/spectral.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/ath/ath11k/spectral.c +0 -0
index 1c5d65bb411f..bfbf905f7507 100644

drivers/net/wireless/ath/ath11k/spectral.c static int ath11k_spectral_process_data(struct ath11k *ar, · line 681

Quoted 6 lines
drivers/net/wireless/ath/ath11k/spectral.c +1 -1
@@ -677,7 +677,7 @@ static int ath11k_spectral_process_data(struct ath11k *ar,
 	u32 data_len, i;
 	u8 sign, tag;
 	int tlv_len, sample_sz;
-	int ret;
+	int ret = 0;
 	bool quit = false;

I try to avoid initialising ret variables so I would like find another way. What about doing this (completely untested!) in the end of the function:

        return 0;

err:

	kfree(fft_sample);
unlock:
	spin_unlock_bh(&ar->spectral.lock);
	return ret;
Signature · 1 line
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
author
#2 lore raw
Quoted 27 lines · Kalle Valo
Quoted 14 lines · Dan Carpenter

There is a success path where "ret" isn't initialized where we never have a ATH11K_SPECTRAL_TAG_SCAN_SEARCH and then ret isn't initialized.

Fixes 9d11b7bff950 ("ath11k: add support for spectral scan")
Signed-off-by Dan Carpenter <[email protected]>
---
 drivers/net/wireless/ath/ath11k/spectral.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/ath/ath11k/spectral.c +0 -0
index 1c5d65bb411f..bfbf905f7507 100644

drivers/net/wireless/ath/ath11k/spectral.c static int ath11k_spectral_process_data(struct ath11k *ar, · line 681

Quoted 6 lines
drivers/net/wireless/ath/ath11k/spectral.c +1 -1
@@ -677,7 +677,7 @@ static int ath11k_spectral_process_data(struct ath11k *ar,
 	u32 data_len, i;
 	u8 sign, tag;
 	int tlv_len, sample_sz;
-	int ret;
+	int ret = 0;
 	bool quit = false;

I try to avoid initialising ret variables so I would like find another way. What about doing this (completely untested!) in the end of the function:

Quoted 7 lines
        return 0;

err:

	kfree(fft_sample);
unlock:
	spin_unlock_bh(&ar->spectral.lock);
	return ret;

I normally avoid it as well... If I were to redo this patch, I would probably do:

	ret = 0;
err:
	kfree(fft_sample);
unlock:
	spin_unlock_bh(&ar->spectral.lock);
	return ret;

Would that be better?

regards,
dan carpenter

#3 lore raw
Quoted 39 lines · Dan Carpenter
Quoted 27 lines · Kalle Valo
Quoted 14 lines · Dan Carpenter

There is a success path where "ret" isn't initialized where we never have a ATH11K_SPECTRAL_TAG_SCAN_SEARCH and then ret isn't initialized.

Fixes 9d11b7bff950 ("ath11k: add support for spectral scan")
Signed-off-by Dan Carpenter <[email protected]>
---
 drivers/net/wireless/ath/ath11k/spectral.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/ath/ath11k/spectral.c +0 -0
index 1c5d65bb411f..bfbf905f7507 100644

drivers/net/wireless/ath/ath11k/spectral.c static int ath11k_spectral_process_data(struct ath11k *ar, · line 681

Quoted 6 lines
drivers/net/wireless/ath/ath11k/spectral.c +1 -1
@@ -677,7 +677,7 @@ static int ath11k_spectral_process_data(struct ath11k *ar,
 	u32 data_len, i;
 	u8 sign, tag;
 	int tlv_len, sample_sz;
-	int ret;
+	int ret = 0;
 	bool quit = false;

I try to avoid initialising ret variables so I would like find another way. What about doing this (completely untested!) in the end of the function:

Quoted 7 lines
        return 0;

err:

	kfree(fft_sample);
unlock:
	spin_unlock_bh(&ar->spectral.lock);
	return ret;

I normally avoid it as well... If I were to redo this patch, I would probably do:

Quoted 8 lines
	ret = 0;
err:
	kfree(fft_sample);
unlock:
	spin_unlock_bh(&ar->spectral.lock);
	return ret;

Would that be better?

Ah, my proposal was buggy. So yes, this is definitely better :) I changed now the patch in the pending branch, please check:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&idX6f8e6779fa75ade112cb48606794e034e0ff1d

Signature · 1 line
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
#4 lore raw
In reply to Dan Carpenter
Quoted 39 lines · Dan Carpenter
Quoted 27 lines · Kalle Valo
Quoted 14 lines · Dan Carpenter

There is a success path where "ret" isn't initialized where we never have a ATH11K_SPECTRAL_TAG_SCAN_SEARCH and then ret isn't initialized.

Fixes 9d11b7bff950 ("ath11k: add support for spectral scan")
Signed-off-by Dan Carpenter <[email protected]>
---
 drivers/net/wireless/ath/ath11k/spectral.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/ath/ath11k/spectral.c +0 -0
index 1c5d65bb411f..bfbf905f7507 100644

drivers/net/wireless/ath/ath11k/spectral.c static int ath11k_spectral_process_data(struct ath11k *ar, · line 681

Quoted 6 lines
drivers/net/wireless/ath/ath11k/spectral.c +1 -1
@@ -677,7 +677,7 @@ static int ath11k_spectral_process_data(struct ath11k *ar,
 	u32 data_len, i;
 	u8 sign, tag;
 	int tlv_len, sample_sz;
-	int ret;
+	int ret = 0;
 	bool quit = false;

I try to avoid initialising ret variables so I would like find another way. What about doing this (completely untested!) in the end of the function:

Quoted 7 lines
        return 0;

err:

	kfree(fft_sample);
unlock:
	spin_unlock_bh(&ar->spectral.lock);
	return ret;

I normally avoid it as well... If I were to redo this patch, I would probably do:

Quoted 8 lines
	ret = 0;
err:
	kfree(fft_sample);
unlock:
	spin_unlock_bh(&ar->spectral.lock);
	return ret;

Would that be better?

Ah, my proposal was buggy. So yes, this is definitely better :) I changed now the patch in the pending branch, please check:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=586f8e6779fa75ade112cb48606794e034e0ff1d

Signature · 6 lines
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

-- 
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k
#5 lore raw
In reply to Dan Carpenter
Quoted 6 lines · Dan Carpenter

There is a success path where "ret" isn't initialized where we never have a ATH11K_SPECTRAL_TAG_SCAN_SEARCH and then ret isn't initialized.

Fixes 9d11b7bff950 ("ath11k: add support for spectral scan")
Signed-off-by Dan Carpenter <[email protected]>
Signed-off-by Kalle Valo <[email protected]>

Patch applied to ath-next branch of ath.git, thanks.

c7187acc3cd0 ath11k: fix uninitialized return in ath11k_spectral_process_data()

Signature · 3 lines
https://patchwork.kernel.org/patch/11614313/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
#6 lore raw
In reply to Dan Carpenter
Quoted 6 lines · Dan Carpenter

There is a success path where "ret" isn't initialized where we never have a ATH11K_SPECTRAL_TAG_SCAN_SEARCH and then ret isn't initialized.

Fixes 9d11b7bff950 ("ath11k: add support for spectral scan")
Signed-off-by Dan Carpenter <[email protected]>
Signed-off-by Kalle Valo <[email protected]>

Patch applied to ath-next branch of ath.git, thanks.

c7187acc3cd0 ath11k: fix uninitialized return in ath11k_spectral_process_data()

Signature · 9 lines
https://patchwork.kernel.org/patch/11614313/

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


-- 
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k