Hello Shaul Triebitz,
The patch 732d06e9d9cf: "iwlwifi: mvm: add station before allocating
a queue" from Jul 10, 2017, leads to the following static checker
warning:
drivers/net/wireless/intel/iwlwifi/mvm/sta.c:1312 iwl_mvm_add_int_sta_common()
error: uninitialized symbol 'status'.
drivers/net/wireless/intel/iwlwifi/mvm/sta.c
1281 static int iwl_mvm_add_int_sta_common(struct iwl_mvm *mvm,
1282 struct iwl_mvm_int_sta *sta,
1283 const u8 *addr,
1284 u16 mac_id, u16 color)
1285 {
1286 struct iwl_mvm_add_sta_cmd cmd;
1287 int ret;
1288 u32 status;
^^^^^^^^^^
1289
1290 lockdep_assert_held(&mvm->mutex);
1291
1292 memset(&cmd, 0, sizeof(cmd));
1293 cmd.sta_id = sta->sta_id;
1294 cmd.mac_id_n_color = cpu_to_le32(FW_CMD_ID_AND_COLOR(mac_id,
1295 color));
1296 if (fw_has_api(&mvm->fw->ucode_capa, IWL_UCODE_TLV_API_STA_TYPE))
1297 cmd.station_type = sta->type;
1298
1299 if (!iwl_mvm_has_new_tx_api(mvm))
1300 cmd.tfd_queue_msk = cpu_to_le32(sta->tfd_queue_msk);
1301 cmd.tid_disable_tx = cpu_to_le16(0xffff);
1302
1303 if (addr)
1304 memcpy(cmd.addr, addr, ETH_ALEN);
1305
1306 ret = iwl_mvm_send_cmd_pdu_status(mvm, ADD_STA,
1307 iwl_mvm_add_sta_cmd_size(mvm),
1308 &cmd, &status);
^^^^^^
1309 if (ret)
1310 return ret;
1311
1312 switch (status & IWL_ADD_STA_STATUS_MASK) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1313 case ADD_STA_SUCCESS:
1314 IWL_DEBUG_INFO(mvm, "Internal station added.\n");
1315 return 0;
1316 default:
1317 ret = -EIO;
1318 IWL_ERR(mvm, "Add internal station failed, status=0x%x\n",
1319 status);
1320 break;
1321 }
1322 return ret;
1323 }
The problem here is this code from iwl_mvm_send_cmd_status()
drivers/net/wireless/intel/iwlwifi/mvm/utils.c
157 cmd->flags |= CMD_WANT_SKB;
158
159 ret = iwl_trans_send_cmd(mvm->trans, cmd);
160 if (ret == -ERFKILL) {
161 /*
162 * The command failed because of RFKILL, don't update
163 * the status, leave it as success and return 0.
164 */
165 return 0;
We return zero without setting "status". Shouldn't we set *status = 0;?
166 } else if (ret) {
167 return ret;
168 }
169
170 pkt = cmd->resp_pkt;
regards,
dan carpenter
T24gRnJpLCAyMDE3LTA5LTAxIGF0IDExOjMwICswMzAwLCBEYW4gQ2FycGVudGVyIHdyb3RlOg0K
PiBIZWxsbyBTaGF1bCBUcmllYml0eiwNCj4gDQo+IFRoZSBwYXRjaCA3MzJkMDZlOWQ5Y2Y6ICJp
d2x3aWZpOiBtdm06IGFkZCBzdGF0aW9uIGJlZm9yZSBhbGxvY2F0aW5nDQo+IGEgcXVldWUiIGZy
b20gSnVsIDEwLCAyMDE3LCBsZWFkcyB0byB0aGUgZm9sbG93aW5nIHN0YXRpYyBjaGVja2VyDQo+
IHdhcm5pbmc6DQo+IA0KPiAJZHJpdmVycy9uZXQvd2lyZWxlc3MvaW50ZWwvaXdsd2lmaS9tdm0v
c3RhLmM6MTMxMiBpd2xfbXZtX2FkZF9pbnRfc3RhX2NvbW1vbigpDQo+IAllcnJvcjogdW5pbml0
aWFsaXplZCBzeW1ib2wgJ3N0YXR1cycuDQo+IA0KPiBkcml2ZXJzL25ldC93aXJlbGVzcy9pbnRl
bC9pd2x3aWZpL212bS9zdGEuYw0KPiAgIDEyODEgIHN0YXRpYyBpbnQgaXdsX212bV9hZGRfaW50
X3N0YV9jb21tb24oc3RydWN0IGl3bF9tdm0gKm12bSwNCj4gICAxMjgyICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgIHN0cnVjdCBpd2xfbXZtX2ludF9zdGEgKnN0YSwNCj4g
ICAxMjgzICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGNvbnN0IHU4ICph
ZGRyLA0KPiAgIDEyODQgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgdTE2
IG1hY19pZCwgdTE2IGNvbG9yKQ0KPiAgIDEyODUgIHsNCj4gICAxMjg2ICAgICAgICAgIHN0cnVj
dCBpd2xfbXZtX2FkZF9zdGFfY21kIGNtZDsNCj4gICAxMjg3ICAgICAgICAgIGludCByZXQ7DQo+
ICAgMTI4OCAgICAgICAgICB1MzIgc3RhdHVzOw0KPiAgICAgICAgICAgICAgICAgXl5eXl5eXl5e
Xg0KPiAgIDEyODkgIA0KPiAgIDEyOTAgICAgICAgICAgbG9ja2RlcF9hc3NlcnRfaGVsZCgmbXZt
LT5tdXRleCk7DQo+ICAgMTI5MSAgDQo+ICAgMTI5MiAgICAgICAgICBtZW1zZXQoJmNtZCwgMCwg
c2l6ZW9mKGNtZCkpOw0KPiAgIDEyOTMgICAgICAgICAgY21kLnN0YV9pZCA9IHN0YS0+c3RhX2lk
Ow0KPiAgIDEyOTQgICAgICAgICAgY21kLm1hY19pZF9uX2NvbG9yID0gY3B1X3RvX2xlMzIoRldf
Q01EX0lEX0FORF9DT0xPUihtYWNfaWQsDQo+ICAgMTI5NSAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGNvbG9yKSk7DQo+ICAgMTI5
NiAgICAgICAgICBpZiAoZndfaGFzX2FwaSgmbXZtLT5mdy0+dWNvZGVfY2FwYSwgSVdMX1VDT0RF
X1RMVl9BUElfU1RBX1RZUEUpKQ0KPiAgIDEyOTcgICAgICAgICAgICAgICAgICBjbWQuc3RhdGlv
bl90eXBlID0gc3RhLT50eXBlOw0KPiAgIDEyOTggIA0KPiAgIDEyOTkgICAgICAgICAgaWYgKCFp
d2xfbXZtX2hhc19uZXdfdHhfYXBpKG12bSkpDQo+ICAgMTMwMCAgICAgICAgICAgICAgICAgIGNt
ZC50ZmRfcXVldWVfbXNrID0gY3B1X3RvX2xlMzIoc3RhLT50ZmRfcXVldWVfbXNrKTsNCj4gICAx
MzAxICAgICAgICAgIGNtZC50aWRfZGlzYWJsZV90eCA9IGNwdV90b19sZTE2KDB4ZmZmZik7DQo+
ICAgMTMwMiAgDQo+ICAgMTMwMyAgICAgICAgICBpZiAoYWRkcikNCj4gICAxMzA0ICAgICAgICAg
ICAgICAgICAgbWVtY3B5KGNtZC5hZGRyLCBhZGRyLCBFVEhfQUxFTik7DQo+ICAgMTMwNSAgDQo+
ICAgMTMwNiAgICAgICAgICByZXQgPSBpd2xfbXZtX3NlbmRfY21kX3BkdV9zdGF0dXMobXZtLCBB
RERfU1RBLA0KPiAgIDEzMDcgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgIGl3bF9tdm1fYWRkX3N0YV9jbWRfc2l6ZShtdm0pLA0KPiAgIDEzMDggICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICZjbWQsICZzdGF0dXMpOw0KPiAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBeXl5eXl4N
Cj4gICAxMzA5ICAgICAgICAgIGlmIChyZXQpDQo+ICAgMTMxMCAgICAgICAgICAgICAgICAgIHJl
dHVybiByZXQ7DQo+ICAgMTMxMSAgDQo+ICAgMTMxMiAgICAgICAgICBzd2l0Y2ggKHN0YXR1cyAm
IElXTF9BRERfU1RBX1NUQVRVU19NQVNLKSB7DQo+ICAgICAgICAgICAgICAgICAgICAgICAgIF5e
Xl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eDQo+ICAgMTMxMyAgICAgICAgICBjYXNlIEFE
RF9TVEFfU1VDQ0VTUzoNCj4gICAxMzE0ICAgICAgICAgICAgICAgICAgSVdMX0RFQlVHX0lORk8o
bXZtLCAiSW50ZXJuYWwgc3RhdGlvbiBhZGRlZC5cbiIpOw0KPiAgIDEzMTUgICAgICAgICAgICAg
ICAgICByZXR1cm4gMDsNCj4gICAxMzE2ICAgICAgICAgIGRlZmF1bHQ6DQo+ICAgMTMxNyAgICAg
ICAgICAgICAgICAgIHJldCA9IC1FSU87DQo+ICAgMTMxOCAgICAgICAgICAgICAgICAgIElXTF9F
UlIobXZtLCAiQWRkIGludGVybmFsIHN0YXRpb24gZmFpbGVkLCBzdGF0dXM9MHgleFxuIiwNCj4g
ICAxMzE5ICAgICAgICAgICAgICAgICAgICAgICAgICBzdGF0dXMpOw0KPiAgIDEzMjAgICAgICAg
ICAgICAgICAgICBicmVhazsNCj4gICAxMzIxICAgICAgICAgIH0NCj4gICAxMzIyICAgICAgICAg
IHJldHVybiByZXQ7DQo+ICAgMTMyMyAgfQ0KPiANCj4gVGhlIHByb2JsZW0gaGVyZSBpcyB0aGlz
IGNvZGUgZnJvbSBpd2xfbXZtX3NlbmRfY21kX3N0YXR1cygpDQo+IA0KPiBkcml2ZXJzL25ldC93
aXJlbGVzcy9pbnRlbC9pd2x3aWZpL212bS91dGlscy5jDQo+ICAgIDE1NyAgICAgICAgICBjbWQt
PmZsYWdzIHw9IENNRF9XQU5UX1NLQjsNCj4gICAgMTU4ICANCj4gICAgMTU5ICAgICAgICAgIHJl
dCA9IGl3bF90cmFuc19zZW5kX2NtZChtdm0tPnRyYW5zLCBjbWQpOw0KPiAgICAxNjAgICAgICAg
ICAgaWYgKHJldCA9PSAtRVJGS0lMTCkgew0KPiAgICAxNjEgICAgICAgICAgICAgICAgICAvKg0K
PiAgICAxNjIgICAgICAgICAgICAgICAgICAgKiBUaGUgY29tbWFuZCBmYWlsZWQgYmVjYXVzZSBv
ZiBSRktJTEwsIGRvbid0IHVwZGF0ZQ0KPiAgICAxNjMgICAgICAgICAgICAgICAgICAgKiB0aGUg
c3RhdHVzLCBsZWF2ZSBpdCBhcyBzdWNjZXNzIGFuZCByZXR1cm4gMC4NCj4gICAgMTY0ICAgICAg
ICAgICAgICAgICAgICovDQo+ICAgIDE2NSAgICAgICAgICAgICAgICAgIHJldHVybiAwOw0KPiAN
Cj4gV2UgcmV0dXJuIHplcm8gd2l0aG91dCBzZXR0aW5nICJzdGF0dXMiLiAgU2hvdWxkbid0IHdl
IHNldCAqc3RhdHVzID0gMDs/DQo+IA0KPiAgICAxNjYgICAgICAgICAgfSBlbHNlIGlmIChyZXQp
IHsNCj4gICAgMTY3ICAgICAgICAgICAgICAgICAgcmV0dXJuIHJldDsNCj4gICAgMTY4ICAgICAg
ICAgIH0NCj4gICAgMTY5ICANCj4gICAgMTcwICAgICAgICAgIHBrdCA9IGNtZC0+cmVzcF9wa3Q7
DQoNClRoZSBjYWxsZXIgc2hvdWxkIHNldCB0aGUgc3RhdHVzIHRvICJzdWNjZXNzIiBiZWZvcmUg
Y2FsbGluZyB0aGlzDQpmdW5jdGlvbi4gIEluIHNvbWUgY2FzZXMgc3VjY2VzcyBpcyBub3QgemVy
byAoZS5nLiB3ZSB1c2UNCkFERF9TVEFfU1VDQ0VTUywgd2hpY2ggaXMgMSwgaW4gc2V2ZXJhbCBw
bGFjZXMpLg0KDQpJJ2xsIHNlbmQgYSBmaXggc3BlY2lmaWMgZm9yIHRoaXMgcGF0Y2ggYW5kIGFu
IGFkZGl0aW9uYWwgb25lIGZvciBvdGhlcg0KcGxhY2VzIHdoZXJlIHdlIGFsc28gY2FsbCB0aGlz
IGZ1bmN0aW9uIHdpdGhvdXQgaW5pdGlhbGl6aW5nIHN0YXR1cy4NCg0KVGhhbmtzIGZvciByZXBv
cnRpbmchDQoNCi0tDQpDaGVlcnMsDQpMdWNhLg==
From: Luca Coelho <[email protected]>
We always need to initialize the status argument to the success case
before calling iwl_mvm_send_cmd_status() or
iwl_mvm_send_cmd_pdu_status() (which calls the former) otherwise we
may get an uninitialized value back. In this case, we use
ADD_STA_SUCCESS as success.
Fixes: 732d06e9d9cf ("iwlwifi: mvm: add station before allocating a queue")
Reported by: Dan Carpenter <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
drivers/net/wireless/intel/iwlwifi/mvm/sta.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
index 411a2055dc45..b476c365d9b4 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
@@ -1285,7 +1285,7 @@ static int iwl_mvm_add_int_sta_common(struct iwl_mvm *mvm,
{
struct iwl_mvm_add_sta_cmd cmd;
int ret;
- u32 status;
+ u32 status = ADD_STA_SUCCESS;
lockdep_assert_held(&mvm->mutex);
--
2.14.1