From: Dan Carpenter <dan.carpenter@oracle.com>
To: hdegoede@redhat.com
Cc: linux-staging@lists.linux.dev
Subject: [bug report] staging: Add rtl8723bs sdio wifi driver
Date: Tue, 5 Oct 2021 12:06:46 +0300 [thread overview]
Message-ID: <20211005090646.GA18431@kili> (raw)
Hello Hans de Goede,
The patch 554c0a3abf21: "staging: Add rtl8723bs sdio wifi driver"
from Mar 29, 2017, leads to the following Smatch static checker
warnings:
drivers/staging/rtl8723bs/core/rtw_security.c:1404 rtw_BIP_verify()
warn: not copying enough bytes for '&le_tmp64' (8 vs 6 bytes)
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:4058 collect_bss_info()
warn: not copying enough bytes for '&le32_tmp' (4 vs 2 bytes)
drivers/staging/rtl8723bs/core/rtw_security.c
1372 u32 rtw_BIP_verify(struct adapter *padapter, u8 *precvframe)
1373 {
1374 struct rx_pkt_attrib *pattrib = &((union recv_frame *)precvframe)->u.hdr.attrib;
1375 u8 *pframe;
1376 u8 *BIP_AAD, *p;
1377 u32 res = _FAIL;
1378 uint len, ori_len;
1379 struct ieee80211_hdr *pwlanhdr;
1380 u8 mic[16];
1381 struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
1382 __le16 le_tmp;
1383 __le64 le_tmp64;
^^^^^^^^^^^^^^^
1384
1385 ori_len = pattrib->pkt_len-WLAN_HDR_A3_LEN+BIP_AAD_SIZE;
1386 BIP_AAD = rtw_zmalloc(ori_len);
1387
1388 if (!BIP_AAD)
1389 return _FAIL;
1390
1391 /* PKT start */
1392 pframe = (unsigned char *)((union recv_frame *)precvframe)->u.hdr.rx_data;
1393 /* mapping to wlan header */
1394 pwlanhdr = (struct ieee80211_hdr *)pframe;
1395 /* save the frame body + MME */
1396 memcpy(BIP_AAD+BIP_AAD_SIZE, pframe+WLAN_HDR_A3_LEN, pattrib->pkt_len-WLAN_HDR_A3_LEN);
1397 /* find MME IE pointer */
1398 p = rtw_get_ie(BIP_AAD+BIP_AAD_SIZE, WLAN_EID_MMIE, &len, pattrib->pkt_len-WLAN_HDR_A3_LEN);
1399 /* Baron */
1400 if (p) {
1401 u16 keyid = 0;
1402 u64 temp_ipn = 0;
1403 /* save packet number */
--> 1404 memcpy(&le_tmp64, p+4, 6);
^^^^^^^^^^^^^^^^^
1405 temp_ipn = le64_to_cpu(le_tmp64);
^^^^^^^^
This code is copying 6 bytes into a u64 and then treating it as little
endian data. The problem is that the last two bytes are uninitialized
garbage data. I think if we set "__le64 le_tmp64 = 0;" at the top that
would probably work, right?
I could have sent a patch but this code is weird enough that I was
hoping for a second opinion.
The bug in collect_bss_info() is a similar uninitialized data issue.
1406 /* BIP packet number should bigger than previous BIP packet */
1407 if (temp_ipn <= pmlmeext->mgnt_80211w_IPN_rx)
1408 goto BIP_exit;
1409
1410 /* copy key index */
1411 memcpy(&le_tmp, p+2, 2);
But this part seems totally wrong again because we haven't incremented
p. p + 10?
1412 keyid = le16_to_cpu(le_tmp);
1413 if (keyid != padapter->securitypriv.dot11wBIPKeyid)
1414 goto BIP_exit;
1415
1416 /* clear the MIC field of MME to zero */
1417 memset(p+2+len-8, 0, 8);
1418
1419 /* conscruct AAD, copy frame control field */
1420 memcpy(BIP_AAD, &pwlanhdr->frame_control, 2);
1421 ClearRetry(BIP_AAD);
1422 ClearPwrMgt(BIP_AAD);
1423 ClearMData(BIP_AAD);
1424 /* conscruct AAD, copy address 1 to address 3 */
1425 memcpy(BIP_AAD+2, pwlanhdr->addr1, 18);
1426
1427 if (omac1_aes_128(padapter->securitypriv.dot11wBIPKey[padapter->securitypriv.dot11wBIPKeyid].skey
1428 , BIP_AAD, ori_len, mic))
1429 goto BIP_exit;
1430
1431 /* MIC field should be last 8 bytes of packet (packet without FCS) */
1432 if (!memcmp(mic, pframe+pattrib->pkt_len-8, 8)) {
1433 pmlmeext->mgnt_80211w_IPN_rx = temp_ipn;
1434 res = _SUCCESS;
1435 } else {
1436 }
1437
1438 } else {
1439 res = RTW_RX_HANDLED;
1440 }
1441 BIP_exit:
1442
1443 kfree(BIP_AAD);
1444 return res;
1445 }
regards,
dan carpenter
next reply other threads:[~2021-10-05 9:08 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-05 9:06 Dan Carpenter [this message]
2021-10-10 10:45 ` [bug report] staging: Add rtl8723bs sdio wifi driver Fabio Aiuto
2021-10-11 12:50 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211005090646.GA18431@kili \
--to=dan.carpenter@oracle.com \
--cc=hdegoede@redhat.com \
--cc=linux-staging@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.