[PATCH] wifi: ath11k: fix few -Wmaybe-uninitialized warnings

[PATCH] wifi: ath11k: fix few -Wmaybe-uninitialized warnings

[Dmitry Antipov]

When compiling with gcc version 14.0.1 20240226 (experimental) and
W=12, I've noticed the following warnings:

drivers/net/wireless/ath/ath11k/mac.c: In function 'ath11k_mac_op_remain_on_channel':
drivers/net/wireless/ath/ath11k/mac.c:9230:12: warning: 'ret' may be used uninitialized
[-Wmaybe-uninitialized]
 9230 |         if (ret)

drivers/net/wireless/ath/ath11k/qmi.c: In function 'ath11k_qmi_load_file_target_mem':
drivers/net/wireless/ath/ath11k/qmi.c:2401:16: warning: 'ret' may be used uninitialized
[-Wmaybe-uninitialized]
 2401 |         return ret;

drivers/net/wireless/ath/ath11k/qmi.c: In function 'ath11k_qmi_load_bdf_qmi':
drivers/net/wireless/ath/ath11k/qmi.c:2494:17: warning: 'fw_entry' may be used uninitialized
[-Wmaybe-uninitialized]
 2494 |                 release_firmware(fw_entry);

And a bunch of them traced to uninitialized fields of the same
variable, e.g.:

drivers/net/wireless/ath/ath11k/spectral.c: In function 'ath11k_spectral_process_data':
drivers/net/wireless/ath/ath11k/spectral.c:700:47: warning: 'summ_rpt.meta.freq1' may
be used uninitialized [-Wmaybe-uninitialized]
  700 |         struct ath11k_spectral_summary_report summ_rpt;

Fix all of the above by using 0 and NULL initializers where appropriate.
Note there are few more (less obvious) -Wmaybe-uninitialized warnings
still remains, but they're hardly possible to fix without running on
a physical hardware. Compile tested oly.

Also noticed by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 drivers/net/wireless/ath/ath11k/mac.c      | 2 +-
 drivers/net/wireless/ath/ath11k/qmi.c      | 4 ++--
 drivers/net/wireless/ath/ath11k/spectral.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index a6a37d67a50a..b89bc7ceaaa7 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -9201,7 +9201,7 @@ static int ath11k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 	struct ath11k *ar = hw->priv;
 	struct ath11k_vif *arvif = ath11k_vif_to_arvif(vif);
 	struct scan_req_params *arg;
-	int ret;
+	int ret = 0;
 	u32 scan_time_msec;
 
 	mutex_lock(&ar->conf_mutex);
diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
index 5006f81f779b..4477f652e068 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -2293,7 +2293,7 @@ static int ath11k_qmi_load_file_target_mem(struct ath11k_base *ab,
 	struct qmi_txn txn;
 	const u8 *temp = data;
 	void __iomem *bdf_addr = NULL;
-	int ret;
+	int ret = 0;
 	u32 remaining = len;
 
 	req = kzalloc(sizeof(*req), GFP_KERNEL);
@@ -2406,7 +2406,7 @@ static int ath11k_qmi_load_bdf_qmi(struct ath11k_base *ab,
 {
 	struct device *dev = ab->dev;
 	char filename[ATH11K_QMI_MAX_BDF_FILE_NAME_SIZE];
-	const struct firmware *fw_entry;
+	const struct firmware *fw_entry = NULL;
 	struct ath11k_board_data bd;
 	u32 fw_size, file_type;
 	int ret = 0, bdf_type;
diff --git a/drivers/net/wireless/ath/ath11k/spectral.c b/drivers/net/wireless/ath/ath11k/spectral.c
index 79e091134515..4c826b539404 100644
--- a/drivers/net/wireless/ath/ath11k/spectral.c
+++ b/drivers/net/wireless/ath/ath11k/spectral.c
@@ -697,7 +697,7 @@ static int ath11k_spectral_process_data(struct ath11k *ar,
 	struct ath11k_base *ab = ar->ab;
 	struct spectral_tlv *tlv;
 	struct spectral_summary_fft_report *summary = NULL;
-	struct ath11k_spectral_summary_report summ_rpt;
+	struct ath11k_spectral_summary_report summ_rpt = { 0 };
 	struct fft_sample_ath11k *fft_sample = NULL;
 	u8 *data;
 	u32 data_len, i;
-- 
2.43.2

Re: [PATCH] wifi: ath11k: fix few -Wmaybe-uninitialized warnings

[Kalle Valo]

Dmitry Antipov <dmantipov@yandex.ru> writes:

> When compiling with gcc version 14.0.1 20240226 (experimental) and
> W=12, I've noticed the following warnings:
>
> drivers/net/wireless/ath/ath11k/mac.c: In function 'ath11k_mac_op_remain_on_channel':
> drivers/net/wireless/ath/ath11k/mac.c:9230:12: warning: 'ret' may be used uninitialized
> [-Wmaybe-uninitialized]
>  9230 |         if (ret)
>
> drivers/net/wireless/ath/ath11k/qmi.c: In function 'ath11k_qmi_load_file_target_mem':
> drivers/net/wireless/ath/ath11k/qmi.c:2401:16: warning: 'ret' may be used uninitialized
> [-Wmaybe-uninitialized]
>  2401 |         return ret;
>
> drivers/net/wireless/ath/ath11k/qmi.c: In function 'ath11k_qmi_load_bdf_qmi':
> drivers/net/wireless/ath/ath11k/qmi.c:2494:17: warning: 'fw_entry' may be used uninitialized
> [-Wmaybe-uninitialized]
>  2494 |                 release_firmware(fw_entry);
>
> And a bunch of them traced to uninitialized fields of the same
> variable, e.g.:
>
> drivers/net/wireless/ath/ath11k/spectral.c: In function 'ath11k_spectral_process_data':
> drivers/net/wireless/ath/ath11k/spectral.c:700:47: warning: 'summ_rpt.meta.freq1' may
> be used uninitialized [-Wmaybe-uninitialized]
>   700 |         struct ath11k_spectral_summary_report summ_rpt;
>
> Fix all of the above by using 0 and NULL initializers where appropriate.
> Note there are few more (less obvious) -Wmaybe-uninitialized warnings
> still remains, but they're hardly possible to fix without running on
> a physical hardware. Compile tested oly.
>
> Also noticed by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>

Did you verify that the warnings are valid?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

Re: [PATCH] wifi: ath11k: fix few -Wmaybe-uninitialized warnings

[Dmitry Antipov]

On 2/28/24 17:01, Kalle Valo wrote:

> 
> Did you verify that the warnings are valid?
> 

Well, hopefully yes. Just in case, gcc 13.2.1 issues an
exactly the same set of -Wmaybe-uninitialized warnings.

Dmitry

Re: [PATCH] wifi: ath11k: fix few -Wmaybe-uninitialized warnings

[Jeff Johnson]

On 2/28/2024 5:14 AM, Dmitry Antipov wrote:
> When compiling with gcc version 14.0.1 20240226 (experimental) and
> W=12, I've noticed the following warnings:
> 
> drivers/net/wireless/ath/ath11k/mac.c: In function 'ath11k_mac_op_remain_on_channel':
> drivers/net/wireless/ath/ath11k/mac.c:9230:12: warning: 'ret' may be used uninitialized
> [-Wmaybe-uninitialized]
>  9230 |         if (ret)
> 
> drivers/net/wireless/ath/ath11k/qmi.c: In function 'ath11k_qmi_load_file_target_mem':
> drivers/net/wireless/ath/ath11k/qmi.c:2401:16: warning: 'ret' may be used uninitialized
> [-Wmaybe-uninitialized]
>  2401 |         return ret;
> 
> drivers/net/wireless/ath/ath11k/qmi.c: In function 'ath11k_qmi_load_bdf_qmi':
> drivers/net/wireless/ath/ath11k/qmi.c:2494:17: warning: 'fw_entry' may be used uninitialized
> [-Wmaybe-uninitialized]
>  2494 |                 release_firmware(fw_entry);
> 
> And a bunch of them traced to uninitialized fields of the same
> variable, e.g.:
> 
> drivers/net/wireless/ath/ath11k/spectral.c: In function 'ath11k_spectral_process_data':
> drivers/net/wireless/ath/ath11k/spectral.c:700:47: warning: 'summ_rpt.meta.freq1' may
> be used uninitialized [-Wmaybe-uninitialized]
>   700 |         struct ath11k_spectral_summary_report summ_rpt;
> 
> Fix all of the above by using 0 and NULL initializers where appropriate.
> Note there are few more (less obvious) -Wmaybe-uninitialized warnings
> still remains, but they're hardly possible to fix without running on
> a physical hardware. Compile tested oly.
> 
> Also noticed by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>  drivers/net/wireless/ath/ath11k/mac.c      | 2 +-
>  drivers/net/wireless/ath/ath11k/qmi.c      | 4 ++--
>  drivers/net/wireless/ath/ath11k/spectral.c | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
> index a6a37d67a50a..b89bc7ceaaa7 100644
> --- a/drivers/net/wireless/ath/ath11k/mac.c
> +++ b/drivers/net/wireless/ath/ath11k/mac.c
> @@ -9201,7 +9201,7 @@ static int ath11k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
>  	struct ath11k *ar = hw->priv;
>  	struct ath11k_vif *arvif = ath11k_vif_to_arvif(vif);
>  	struct scan_req_params *arg;
> -	int ret;
> +	int ret = 0;

NAK
the only time ret would be uninitialized is if the initial switch()
processed an unknown state, in which case we should stop processing, not
process as successful.

the correct fix for this is to add a default: to the switch() which
warns and sets ret to an appropriate errno

Please submit as a separate patch

>  	u32 scan_time_msec;
>  
>  	mutex_lock(&ar->conf_mutex);
> diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
> index 5006f81f779b..4477f652e068 100644
> --- a/drivers/net/wireless/ath/ath11k/qmi.c
> +++ b/drivers/net/wireless/ath/ath11k/qmi.c
> @@ -2293,7 +2293,7 @@ static int ath11k_qmi_load_file_target_mem(struct ath11k_base *ab,
>  	struct qmi_txn txn;
>  	const u8 *temp = data;
>  	void __iomem *bdf_addr = NULL;
> -	int ret;
> +	int ret = 0;

I hate this since the only time ret would be uninitialized is if len is
0 (and hence we never execute the while(remaining) loop), but nothing
ever calls this static function with len == 0

but the alternative to this is to add a guard check and that seems like
overkill, so this one is ok

>  	u32 remaining = len;
>  
>  	req = kzalloc(sizeof(*req), GFP_KERNEL);
> @@ -2406,7 +2406,7 @@ static int ath11k_qmi_load_bdf_qmi(struct ath11k_base *ab,
>  {
>  	struct device *dev = ab->dev;
>  	char filename[ATH11K_QMI_MAX_BDF_FILE_NAME_SIZE];
> -	const struct firmware *fw_entry;
> +	const struct firmware *fw_entry = NULL;
>  	struct ath11k_board_data bd;
>  	u32 fw_size, file_type;
>  	int ret = 0, bdf_type;
> diff --git a/drivers/net/wireless/ath/ath11k/spectral.c b/drivers/net/wireless/ath/ath11k/spectral.c
> index 79e091134515..4c826b539404 100644
> --- a/drivers/net/wireless/ath/ath11k/spectral.c
> +++ b/drivers/net/wireless/ath/ath11k/spectral.c
> @@ -697,7 +697,7 @@ static int ath11k_spectral_process_data(struct ath11k *ar,
>  	struct ath11k_base *ab = ar->ab;
>  	struct spectral_tlv *tlv;
>  	struct spectral_summary_fft_report *summary = NULL;
> -	struct ath11k_spectral_summary_report summ_rpt;
> +	struct ath11k_spectral_summary_report summ_rpt = { 0 };

prefer just {} since that works correctly if we ever want to add a
non-scalar as the first member

>  	struct fft_sample_ath11k *fft_sample = NULL;
>  	u8 *data;
>  	u32 data_len, i;

[PATCH 1/3] [v2] wifi: ath11k: use ath11k_mac_get_ar_by_pdev_id() consistently

[Dmitry Antipov]

Since 'ath11k_mac_get_ar_by_pdev_id()' can return NULL, check
the return value in 'ath11k_wmi_tlv_rssi_chain_parse()' as well
as in 'ath11k_wmi_tlv_fw_stats_data_parse()', and return -EINVAL
in case of error. Compile tested only.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v2: aggregate to the series
---
 drivers/net/wireless/ath/ath11k/wmi.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/wireless/ath/ath11k/wmi.c b/drivers/net/wireless/ath/ath11k/wmi.c
index 34ab9631ff36..2d93e4e78a37 100644
--- a/drivers/net/wireless/ath/ath11k/wmi.c
+++ b/drivers/net/wireless/ath/ath11k/wmi.c
@@ -6498,6 +6498,12 @@ static int ath11k_wmi_tlv_rssi_chain_parse(struct ath11k_base *ab,
 	rcu_read_lock();
 
 	ar = ath11k_mac_get_ar_by_pdev_id(ab, ev->pdev_id);
+	if (!ar) {
+		ath11k_warn(ab, "%s: invalid pdev_id %d\n",
+			    __func__, ev->pdev_id);
+		ret = -EINVAL;
+		goto exit;
+	}
 	stats->stats_id = WMI_REQUEST_RSSI_PER_CHAIN_STAT;
 
 	ath11k_dbg(ab, ATH11K_DBG_WMI,
@@ -6570,6 +6576,12 @@ static int ath11k_wmi_tlv_fw_stats_data_parse(struct ath11k_base *ab,
 	rcu_read_lock();
 
 	ar = ath11k_mac_get_ar_by_pdev_id(ab, ev->pdev_id);
+	if (!ar) {
+		ath11k_warn(ab, "%s: invalid pdev_id %d\n",
+			    __func__, ev->pdev_id);
+		ret = -EINVAL;
+		goto exit;
+	}
 
 	for (i = 0; i < ev->num_pdev_stats; i++) {
 		const struct wmi_pdev_stats *src;
-- 
2.44.0

[PATCH 2/3] [v2] wifi: ath11k: handle unknown scan state in ath11k_mac_op_remain_on_channel()

[Dmitry Antipov]

In 'ath11k_mac_op_remain_on_channel()', add fallback default to
handle an unknown scan state with -EINVAL. Compile tested only.

Initially found by Linux Verification Center (linuxtesting.org)
with SVACE (and reported as an attempt to use uninitialized
variable).

Suggested-by: Jeff Johnson <quic_jjohnson@quicinc.com>
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v2: prefer fallback branch over dummy initializer (Jeff Johnson)
---
 drivers/net/wireless/ath/ath11k/mac.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index a6a37d67a50a..47d3d5fd0423 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -9224,6 +9224,11 @@ static int ath11k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 	case ATH11K_SCAN_ABORTING:
 		ret = -EBUSY;
 		break;
+	default:
+		ath11k_warn(ar->ab, "%s: unexpected scan state: %d\n",
+			    __func__, ar->scan.state);
+		ret = -EINVAL;
+		break;
 	}
 	spin_unlock_bh(&ar->data_lock);
 
-- 
2.44.0

[PATCH 3/3] [v2] wifi: ath11k: fix few -Wmaybe-uninitialized warnings

[Dmitry Antipov]

When compiling with gcc version 14.0.1 20240226 (experimental) and
W=12, I've noticed the following warnings:

drivers/net/wireless/ath/ath11k/qmi.c: In function 'ath11k_qmi_load_file_target_mem':
drivers/net/wireless/ath/ath11k/qmi.c:2401:16: warning: 'ret' may be used uninitialized
[-Wmaybe-uninitialized]
 2401 |         return ret;

drivers/net/wireless/ath/ath11k/qmi.c: In function 'ath11k_qmi_load_bdf_qmi':
drivers/net/wireless/ath/ath11k/qmi.c:2494:17: warning: 'fw_entry' may be used uninitialized
[-Wmaybe-uninitialized]
 2494 |                 release_firmware(fw_entry);

And a bunch of them traced to an uninitialized fields of the same
variable, e.g.:

drivers/net/wireless/ath/ath11k/spectral.c: In function 'ath11k_spectral_process_data':
drivers/net/wireless/ath/ath11k/spectral.c:700:47: warning: 'summ_rpt.meta.freq1' may
be used uninitialized [-Wmaybe-uninitialized]
  700 |         struct ath11k_spectral_summary_report summ_rpt;

Fix all of the above by using 0, NULL, and {} initializers, respectively.
Note there are few more (less obvious) -Wmaybe-uninitialized warnings
still remains, but they're hardly possible to fix without running on
a physical hardware. Compile tested only.

Also noticed by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v2: use {} initializer (Jeff Johnson) and aggregate to the series
---
 drivers/net/wireless/ath/ath11k/qmi.c      | 4 ++--
 drivers/net/wireless/ath/ath11k/spectral.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
index 5006f81f779b..4477f652e068 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -2293,7 +2293,7 @@ static int ath11k_qmi_load_file_target_mem(struct ath11k_base *ab,
 	struct qmi_txn txn;
 	const u8 *temp = data;
 	void __iomem *bdf_addr = NULL;
-	int ret;
+	int ret = 0;
 	u32 remaining = len;
 
 	req = kzalloc(sizeof(*req), GFP_KERNEL);
@@ -2406,7 +2406,7 @@ static int ath11k_qmi_load_bdf_qmi(struct ath11k_base *ab,
 {
 	struct device *dev = ab->dev;
 	char filename[ATH11K_QMI_MAX_BDF_FILE_NAME_SIZE];
-	const struct firmware *fw_entry;
+	const struct firmware *fw_entry = NULL;
 	struct ath11k_board_data bd;
 	u32 fw_size, file_type;
 	int ret = 0, bdf_type;
diff --git a/drivers/net/wireless/ath/ath11k/spectral.c b/drivers/net/wireless/ath/ath11k/spectral.c
index 79e091134515..9834e7dc5120 100644
--- a/drivers/net/wireless/ath/ath11k/spectral.c
+++ b/drivers/net/wireless/ath/ath11k/spectral.c
@@ -697,7 +697,7 @@ static int ath11k_spectral_process_data(struct ath11k *ar,
 	struct ath11k_base *ab = ar->ab;
 	struct spectral_tlv *tlv;
 	struct spectral_summary_fft_report *summary = NULL;
-	struct ath11k_spectral_summary_report summ_rpt;
+	struct ath11k_spectral_summary_report summ_rpt = {};
 	struct fft_sample_ath11k *fft_sample = NULL;
 	u8 *data;
 	u32 data_len, i;
-- 
2.44.0

Re: [PATCH 1/3] [v2] wifi: ath11k: use ath11k_mac_get_ar_by_pdev_id() consistently

[Jeff Johnson]

On 2/29/2024 12:40 AM, Dmitry Antipov wrote:
> Since 'ath11k_mac_get_ar_by_pdev_id()' can return NULL, check
> the return value in 'ath11k_wmi_tlv_rssi_chain_parse()' as well
> as in 'ath11k_wmi_tlv_fw_stats_data_parse()', and return -EINVAL
> in case of error. Compile tested only.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
> v2: aggregate to the series

In the future please don't use In-Reply-To: when submitting a new
version of a patch/series. Each version should start its own thread.
Having In-Reference-To: is OK

> ---
>  drivers/net/wireless/ath/ath11k/wmi.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/wmi.c b/drivers/net/wireless/ath/ath11k/wmi.c
> index 34ab9631ff36..2d93e4e78a37 100644
> --- a/drivers/net/wireless/ath/ath11k/wmi.c
> +++ b/drivers/net/wireless/ath/ath11k/wmi.c
> @@ -6498,6 +6498,12 @@ static int ath11k_wmi_tlv_rssi_chain_parse(struct ath11k_base *ab,
>  	rcu_read_lock();
>  
>  	ar = ath11k_mac_get_ar_by_pdev_id(ab, ev->pdev_id);
> +	if (!ar) {
> +		ath11k_warn(ab, "%s: invalid pdev_id %d\n",
> +			    __func__, ev->pdev_id);

it is currently not the convention to use __func__ in ath11k logs.

there are currently 6 instances where the NULL check is present, and
each has a custom log message

I personally would be ok having this new format, but want to get Kalle's
approval for that. But if we do that I'd also want a follow-up patch to
modify those 6 existing logs.

FWIW in all of ath11k there are only 3 instances of __func__:
mac.c:                     "%s: rsn ie found\n", __func__);
mac.c:                     "%s: wpa ie found\n", __func__);
wmi.c:                     __func__);

> +		ret = -EINVAL;
> +		goto exit;
> +	}
>  	stats->stats_id = WMI_REQUEST_RSSI_PER_CHAIN_STAT;
>  
>  	ath11k_dbg(ab, ATH11K_DBG_WMI,
> @@ -6570,6 +6576,12 @@ static int ath11k_wmi_tlv_fw_stats_data_parse(struct ath11k_base *ab,
>  	rcu_read_lock();
>  
>  	ar = ath11k_mac_get_ar_by_pdev_id(ab, ev->pdev_id);
> +	if (!ar) {
> +		ath11k_warn(ab, "%s: invalid pdev_id %d\n",
> +			    __func__, ev->pdev_id);
> +		ret = -EINVAL;
> +		goto exit;
> +	}
>  
>  	for (i = 0; i < ev->num_pdev_stats; i++) {
>  		const struct wmi_pdev_stats *src;

The actual logic is correct.

/jeff

Re: [PATCH 2/3] [v2] wifi: ath11k: handle unknown scan state in ath11k_mac_op_remain_on_channel()

[Jeff Johnson]

On 2/29/2024 12:40 AM, Dmitry Antipov wrote:
> In 'ath11k_mac_op_remain_on_channel()', add fallback default to
> handle an unknown scan state with -EINVAL. Compile tested only.
> 
> Initially found by Linux Verification Center (linuxtesting.org)
> with SVACE (and reported as an attempt to use uninitialized
> variable).
> 
> Suggested-by: Jeff Johnson <quic_jjohnson@quicinc.com>
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
> v2: prefer fallback branch over dummy initializer (Jeff Johnson)
> ---
>  drivers/net/wireless/ath/ath11k/mac.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
> index a6a37d67a50a..47d3d5fd0423 100644
> --- a/drivers/net/wireless/ath/ath11k/mac.c
> +++ b/drivers/net/wireless/ath/ath11k/mac.c
> @@ -9224,6 +9224,11 @@ static int ath11k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
>  	case ATH11K_SCAN_ABORTING:
>  		ret = -EBUSY;
>  		break;
> +	default:
> +		ath11k_warn(ar->ab, "%s: unexpected scan state: %d\n",
> +			    __func__, ar->scan.state);

again let's wait for Kalle to comment on use of __func__

> +		ret = -EINVAL;
> +		break;
>  	}
>  	spin_unlock_bh(&ar->data_lock);
>  

Re: [PATCH 3/3] [v2] wifi: ath11k: fix few -Wmaybe-uninitialized warnings

[Jeff Johnson]

On 2/29/2024 12:40 AM, Dmitry Antipov wrote:
> When compiling with gcc version 14.0.1 20240226 (experimental) and
> W=12, I've noticed the following warnings:
> 
> drivers/net/wireless/ath/ath11k/qmi.c: In function 'ath11k_qmi_load_file_target_mem':
> drivers/net/wireless/ath/ath11k/qmi.c:2401:16: warning: 'ret' may be used uninitialized
> [-Wmaybe-uninitialized]
>  2401 |         return ret;
> 
> drivers/net/wireless/ath/ath11k/qmi.c: In function 'ath11k_qmi_load_bdf_qmi':
> drivers/net/wireless/ath/ath11k/qmi.c:2494:17: warning: 'fw_entry' may be used uninitialized
> [-Wmaybe-uninitialized]
>  2494 |                 release_firmware(fw_entry);
> 
> And a bunch of them traced to an uninitialized fields of the same
> variable, e.g.:
> 
> drivers/net/wireless/ath/ath11k/spectral.c: In function 'ath11k_spectral_process_data':
> drivers/net/wireless/ath/ath11k/spectral.c:700:47: warning: 'summ_rpt.meta.freq1' may
> be used uninitialized [-Wmaybe-uninitialized]

looks like a false positive since ath11k_spectral_pull_summary() always
sets the entire meta struct:
	memcpy(&report->meta, meta, sizeof(*meta));

>   700 |         struct ath11k_spectral_summary_report summ_rpt;
> 
> Fix all of the above by using 0, NULL, and {} initializers, respectively.
> Note there are few more (less obvious) -Wmaybe-uninitialized warnings
> still remains, but they're hardly possible to fix without running on
> a physical hardware. Compile tested only.
> 
> Also noticed by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
> v2: use {} initializer (Jeff Johnson) and aggregate to the series
> ---
>  drivers/net/wireless/ath/ath11k/qmi.c      | 4 ++--
>  drivers/net/wireless/ath/ath11k/spectral.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
> index 5006f81f779b..4477f652e068 100644
> --- a/drivers/net/wireless/ath/ath11k/qmi.c
> +++ b/drivers/net/wireless/ath/ath11k/qmi.c
> @@ -2293,7 +2293,7 @@ static int ath11k_qmi_load_file_target_mem(struct ath11k_base *ab,
>  	struct qmi_txn txn;
>  	const u8 *temp = data;
>  	void __iomem *bdf_addr = NULL;
> -	int ret;
> +	int ret = 0;

as previously discussed this one I'll begrudgingly take.
please submit as a stand-alone patch

>  	u32 remaining = len;
>  
>  	req = kzalloc(sizeof(*req), GFP_KERNEL);
> @@ -2406,7 +2406,7 @@ static int ath11k_qmi_load_bdf_qmi(struct ath11k_base *ab,
>  {
>  	struct device *dev = ab->dev;
>  	char filename[ATH11K_QMI_MAX_BDF_FILE_NAME_SIZE];
> -	const struct firmware *fw_entry;
> +	const struct firmware *fw_entry = NULL;

note that this also seems to be fixing a false positive.

the "maybe uninitialized" reference is:

out_qmi_cal:
	if (!ab->qmi.target.eeprom_caldata)
		release_firmware(fw_entry);

fw_entry is currently assigned:
	if (ab->qmi.target.eeprom_caldata) {
		...
	} else {
		snprintf(filename, sizeof(filename), "cal-%s-%s.bin",
			 ath11k_bus_str(ab->hif.bus), dev_name(dev));
		fw_entry = ath11k_core_firmware_request(ab, filename);
	}

So unless I'm missing something it is always the case that fw_entry will
be initialized when ab->qmi.target.eeprom_caldata is not set.

>  	struct ath11k_board_data bd;
>  	u32 fw_size, file_type;
>  	int ret = 0, bdf_type;
> diff --git a/drivers/net/wireless/ath/ath11k/spectral.c b/drivers/net/wireless/ath/ath11k/spectral.c
> index 79e091134515..9834e7dc5120 100644
> --- a/drivers/net/wireless/ath/ath11k/spectral.c
> +++ b/drivers/net/wireless/ath/ath11k/spectral.c
> @@ -697,7 +697,7 @@ static int ath11k_spectral_process_data(struct ath11k *ar,
>  	struct ath11k_base *ab = ar->ab;
>  	struct spectral_tlv *tlv;
>  	struct spectral_summary_fft_report *summary = NULL;
> -	struct ath11k_spectral_summary_report summ_rpt;
> +	struct ath11k_spectral_summary_report summ_rpt = {};
>  	struct fft_sample_ath11k *fft_sample = NULL;
>  	u8 *data;
>  	u32 data_len, i;

so NAK on changes which "fix" false positives.