Re: [PATCH v3 2/3] ath11k: add configure country code for QCA6390 and WCN6855

From: Dan Carpenter <dan.carpenter@oracle.com>
To: Wen Gong <quic_wgong@quicinc.com>
Cc: ath11k@lists.infradead.org, linux-wireless@vger.kernel.org
Subject: Re: [PATCH v3 2/3] ath11k: add configure country code for QCA6390 and WCN6855
Date: Thu, 6 Jan 2022 10:18:12 +0300	[thread overview]
Message-ID: <20220106071812.GA3897@kili> (raw)
In-Reply-To: <20211201071745.17746-3-quic_wgong@quicinc.com>

On Wed, Dec 01, 2021 at 02:17:44AM -0500, Wen Gong wrote:
> +int ath11k_wmi_send_set_current_country_cmd(struct ath11k *ar,
> +					    struct wmi_set_current_country_params *param)
> +{
> +	struct ath11k_pdev_wmi *wmi = ar->wmi;
> +	struct wmi_set_current_country_cmd *cmd;
> +	struct sk_buff *skb;
> +	int ret;
> +
> +	skb = ath11k_wmi_alloc_skb(wmi->wmi_ab, sizeof(*cmd));
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	cmd = (struct wmi_set_current_country_cmd *)skb->data;
> +	cmd->tlv_header =
> +		FIELD_PREP(WMI_TLV_TAG, WMI_TAG_SET_CURRENT_COUNTRY_CMD) |
> +		FIELD_PREP(WMI_TLV_LEN, sizeof(*cmd) - TLV_HDR_SIZE);
> +
> +	cmd->pdev_id = ar->pdev->pdev_id;
> +	memcpy(&cmd->new_alpha2, &param->alpha2, 3);

cmd->new_alpha2 is a u32 in this driver.  (In a different driver it is a
3 byte array).  So this will leave the last byte as random garbage.

Probably harmless but it leads to a Smatch static checker warning.

drivers/net/wireless/ath/ath11k/wmi.c:2837 ath11k_wmi_send_set_current_country_cmd()
warn: not copying enough bytes for '&cmd->new_alpha2' (4 vs 3 bytes)

> +	ret = ath11k_wmi_cmd_send(wmi, skb, WMI_SET_CURRENT_COUNTRY_CMDID);
> +
> +	ath11k_dbg(ar->ab, ATH11K_DBG_WMI,
> +		   "set current country pdev id %d alpha2 %c%c\n",
> +		   ar->pdev->pdev_id,
> +		   param->alpha2[0],
> +		   param->alpha2[1]);
> +
> +	if (ret) {
> +		ath11k_warn(ar->ab,
> +			    "failed to send WMI_SET_CURRENT_COUNTRY_CMDID: %d\n", ret);
> +		dev_kfree_skb(skb);
> +	}
> +
> +	return ret;
> +}

regards,
dan carpenter