All of lore.kernel.org
 help / color / mirror / Atom feed
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

             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.