| Message ID | 20240229084031.51957-1-dmantipov@yandex.ru (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [1/3,v2] wifi: ath11k: use ath11k_mac_get_ar_by_pdev_id() consistently | expand |
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
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;
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(+)