Hello Jia-Ju Bai,
The patch 4f68ef64cd7f: "cw1200: Fix concurrency use-after-free bugs
in cw1200_hw_scan()" from Dec 14, 2018, leads to the following static
checker warning:
drivers/net/wireless/st/cw1200/scan.c:127 cw1200_hw_scan() warn: inconsistent
returns 'sem:&priv->scan.lock'.
Locked on: line 88
line 127
Unlocked on: line 66
line 70
drivers/net/wireless/st/cw1200/scan.c:268 cw1200_scan_work() warn: inconsistent
returns 'sem:&priv->scan.lock'.
Locked on: line 262
line 268
Unlocked on: line 196
(I don't really understand the locking in cw1200_scan_work() well enough
to say what's supposed to happen but the first warning seems like a bug).
drivers/net/wireless/st/cw1200/scan.c
54 int cw1200_hw_scan(struct ieee80211_hw *hw,
55 struct ieee80211_vif *vif,
56 struct ieee80211_scan_request *hw_req)
57 {
58 struct cw1200_common *priv = hw->priv;
59 struct cfg80211_scan_request *req = &hw_req->req;
60 struct wsm_template_frame frame = {
61 .frame_type = WSM_FRAME_TYPE_PROBE_REQUEST,
62 };
63 int i, ret;
64
65 if (!priv->vif)
66 return -EINVAL;
67
68 /* Scan when P2P_GO corrupt firmware MiniAP mode */
69 if (priv->join_status == CW1200_JOIN_STATUS_AP)
70 return -EOPNOTSUPP;
71
72 if (req->n_ssids == 1 && !req->ssids[0].ssid_len)
73 req->n_ssids = 0;
74
75 wiphy_dbg(hw->wiphy, "[SCAN] Scan request for %d SSIDs.\n",
76 req->n_ssids);
77
78 if (req->n_ssids > WSM_SCAN_MAX_NUM_OF_SSIDS)
79 return -EINVAL;
80
81 /* will be unlocked in cw1200_scan_work() */
82 down(&priv->scan.lock);
^^^^^^^^^^^^^^^^^^^^^^
83 mutex_lock(&priv->conf_mutex);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
84
85 frame.skb = ieee80211_probereq_get(hw, priv->vif->addr, NULL, 0,
86 req->ie_len);
87 if (!frame.skb)
88 return -ENOMEM;
^^^^^^^^^^^^^^^
Need to drop the locks before returning.
89
90 if (req->ie_len)
91 skb_put_data(frame.skb, req->ie, req->ie_len);
92
93 ret = wsm_set_template_frame(priv, &frame);
94 if (!ret) {
95 /* Host want to be the probe responder. */
96 ret = wsm_set_probe_responder(priv, true);
97 }
98 if (ret) {
99 dev_kfree_skb(frame.skb);
100 mutex_unlock(&priv->conf_mutex);
101 up(&priv->scan.lock);
102 return ret;
103 }
104
105 wsm_lock_tx(priv);
106
107 BUG_ON(priv->scan.req);
108 priv->scan.req = req;
109 priv->scan.n_ssids = 0;
110 priv->scan.status = 0;
111 priv->scan.begin = &req->channels[0];
112 priv->scan.curr = priv->scan.begin;
113 priv->scan.end = &req->channels[req->n_channels];
114 priv->scan.output_power = priv->output_power;
115
116 for (i = 0; i < req->n_ssids; ++i) {
117 struct wsm_ssid *dst =
&priv->scan.ssids[priv->scan.n_ssids];
118 memcpy(&dst->ssid[0], req->ssids[i].ssid,
sizeof(dst->ssid));
119 dst->length = req->ssids[i].ssid_len;
120 ++priv->scan.n_ssids;
121 }
122
123 if (frame.skb)
124 dev_kfree_skb(frame.skb);
125 mutex_unlock(&priv->conf_mutex);
126 queue_work(priv->workqueue, &priv->scan.work);
127 return 0;
128 }
regards,
dan carpenter
