Received-SPF: pass (google.com: domain of linux-wireless+bounces-8264-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Wed, 29 May 2024 17:32:52 +0300
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Kang Yang <quic_kangyang@quicinc.com>
Cc: linux-wireless@vger.kernel.org, ath11k@lists.infradead.org
Subject: [bug report] wifi: ath11k: add P2P IE in beacon template
Message-ID: <2d277abd-5e7b-4da0-80e0-52bd96337f6e@moroto.mountain>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hello Kang Yang,

Commit 3a415daa3e8b ("wifi: ath11k: add P2P IE in beacon template")
from Feb 28, 2024 (linux-next), leads to the following Smatch static
checker warning:

	drivers/net/wireless/ath/ath11k/wmi.c:1742 ath11k_wmi_p2p_go_bcn_ie()
	warn: sleeping in atomic context

drivers/net/wireless/ath/ath11k/wmi.c
    1712 int ath11k_wmi_p2p_go_bcn_ie(struct ath11k *ar, u32 vdev_id,
    1713                              const u8 *p2p_ie)
    1714 {
    1715         struct ath11k_pdev_wmi *wmi = ar->wmi;
    1716         struct wmi_p2p_go_set_beacon_ie_cmd *cmd;
    1717         size_t p2p_ie_len, aligned_len;
    1718         struct wmi_tlv *tlv;
    1719         struct sk_buff *skb;
    1720         int ret, len;
    1721 
    1722         p2p_ie_len = p2p_ie[1] + 2;
    1723         aligned_len = roundup(p2p_ie_len, 4);
    1724 
    1725         len = sizeof(*cmd) + TLV_HDR_SIZE + aligned_len;
    1726 
    1727         skb = ath11k_wmi_alloc_skb(wmi->wmi_ab, len);
    1728         if (!skb)
    1729                 return -ENOMEM;
    1730 
    1731         cmd = (struct wmi_p2p_go_set_beacon_ie_cmd *)skb->data;
    1732         cmd->tlv_header = FIELD_PREP(WMI_TLV_TAG, WMI_TAG_P2P_GO_SET_BEACON_IE) |
    1733                           FIELD_PREP(WMI_TLV_LEN, sizeof(*cmd) - TLV_HDR_SIZE);
    1734         cmd->vdev_id = vdev_id;
    1735         cmd->ie_buf_len = p2p_ie_len;
    1736 
    1737         tlv = (struct wmi_tlv *)cmd->tlv;
    1738         tlv->header = FIELD_PREP(WMI_TLV_TAG, WMI_TAG_ARRAY_BYTE) |
    1739                       FIELD_PREP(WMI_TLV_LEN, aligned_len);
    1740         memcpy(tlv->value, p2p_ie, p2p_ie_len);
    1741 
--> 1742         ret = ath11k_wmi_cmd_send(wmi, skb, WMI_P2P_GO_SET_BEACON_IE);
                       ^^^^^^^^^^^^^^^^^^^
This is a might_sleep() function.

    1743         if (ret) {
    1744                 ath11k_warn(ar->ab, "failed to send WMI_P2P_GO_SET_BEACON_IE\n");
    1745                 dev_kfree_skb(skb);
    1746         }
    1747 
    1748         return ret;
    1749 }

The problematic call tree is:

ath11k_bcn_tx_status_event() <- disables preempt
-> ath11k_mac_bcn_tx_event()
   -> ath11k_mac_setup_bcn_tmpl()
      -> ath11k_mac_setup_bcn_tmpl_ema()
      -> ath11k_mac_setup_bcn_tmpl_mbssid()
         -> ath11k_mac_set_vif_params()
            -> ath11k_mac_setup_bcn_p2p_ie()
               -> ath11k_wmi_p2p_go_bcn_ie()

The ath11k_bcn_tx_status_event() function takes rcu_read_lock() which
disables preemption.  I don't know the code well enough to say if this
is a real bug...  If it's a false positive, just ignore it.  These are
one time emails.

See my blog for more details.
https://staticthinking.wordpress.com/2024/05/24/sleeping-in-atomic-warnings/

regards,
dan carpenter