On 12/12/2024 1:26 PM, Kalle Valo wrote:
Karthikeyan Periyasamy <quic_periyasa@xxxxxxxxxxx> writes:
Currently, the uninitialized variable 'ab' is accessed in the
ath12k_mac_allocate() function. Initialize 'ab' with the first radio device
present in the hardware abstraction handle (ah). Additionally, move the
default setting procedure from the pdev mapping iteration to the total
radio calculating iteration for better code readability. Perform the
maximum radio validation check for total_radio to ensure that both num_hw
and radio_per_hw are validated indirectly, as these variables are derived
from total_radio. This also fixes the below Smatch static checker warning.
Smatch warning:
ath12k_mac_allocate() error: uninitialized symbol 'ab'
Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
Fixes: a343d97f27f5 ("wifi: ath12k: move struct ath12k_hw from per device to group")
Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@xxxxxxxxxxx>
---
drivers/net/wireless/ath/ath12k/mac.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 5cdc1c38b049..98b2f853d243 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -10962,8 +10962,20 @@ int ath12k_mac_allocate(struct ath12k_hw_group *ag)
u8 radio_per_hw;
total_radio = 0;
- for (i = 0; i < ag->num_devices; i++)
- total_radio += ag->ab[i]->num_radios;
+ for (i = 0; i < ag->num_devices; i++) {
+ ab = ag->ab[i];
+ if (!ab)
+ continue;
+
+ ath12k_mac_set_device_defaults(ab);
+ total_radio += ab->num_radios;
+ }
+
+ if (!total_radio)
+ return -EINVAL;
'total_radio == 0' is more readable as it's a counter. Also please add ath12k_warn()
This condition came due to no device handle (ab) present in the
ath12k_hw_group (ag). ath12k_warn() cannot be used since no device (ab)
present here. Additionally, ath12k_hw_warn() also cannot be used here
since ath12k_hw (ah) also not created.
In future, we can introduce device (ab) and hardware (ah) agnostic
warning helper function ath12k*dbg() for this usecase.
+
+ if (WARN_ON(total_radio > ATH12K_GROUP_MAX_RADIO))
+ return -ENOSPC;
BTW ath12k_warn() is preferred over WARN_ON(), but this is just for the
future as this WARN_ON() was already there before.
Sure
/* All pdev get combined and register as single wiphy based on
* hardware group which participate in multi-link operation else
@@ -10976,14 +10988,16 @@ int ath12k_mac_allocate(struct ath12k_hw_group *ag)
num_hw = total_radio / radio_per_hw;
- if (WARN_ON(num_hw >= ATH12K_GROUP_MAX_RADIO))
- return -ENOSPC;
-
ag->num_hw = 0;
device_id = 0;
mac_id = 0;
for (i = 0; i < num_hw; i++) {
for (j = 0; j < radio_per_hw; j++) {
+ if (device_id >= ag->num_devices || !ag->ab[device_id]) {
+ ret = -ENOSPC;
+ goto err;
+ }
ath12k_warn()
same here.
--
Karthikeyan Periyasamy
--
கார்த்திகேயன் பெரியசாமி
