wifi: ath9k: fix fortify warnings
authorDmitry Antipov <dmantipov@yandex.ru>
Mon, 24 Jul 2023 10:11:07 +0000 (13:11 +0300)
committerKalle Valo <quic_kvalo@quicinc.com>
Tue, 25 Jul 2023 14:28:03 +0000 (17:28 +0300)
When compiling with gcc 13.1 and CONFIG_FORTIFY_SOURCE=y,
I've noticed the following:

In function ‘fortify_memcpy_chk’,
    inlined from ‘ath_tx_complete_aggr’ at drivers/net/wireless/ath/ath9k/xmit.c:556:4,
    inlined from ‘ath_tx_process_buffer’ at drivers/net/wireless/ath/ath9k/xmit.c:773:3:
./include/linux/fortify-string.h:529:25: warning: call to ‘__read_overflow2_field’
declared with attribute warning: detected read beyond size of field (2nd parameter);
maybe use struct_group()? [-Wattribute-warning]
  529 |                         __read_overflow2_field(q_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In function ‘fortify_memcpy_chk’,
    inlined from ‘ath_tx_count_frames’ at drivers/net/wireless/ath/ath9k/xmit.c:473:3,
    inlined from ‘ath_tx_complete_aggr’ at drivers/net/wireless/ath/ath9k/xmit.c:572:2,
    inlined from ‘ath_tx_process_buffer’ at drivers/net/wireless/ath/ath9k/xmit.c:773:3:
./include/linux/fortify-string.h:529:25: warning: call to ‘__read_overflow2_field’
declared with attribute warning: detected read beyond size of field (2nd parameter);
maybe use struct_group()? [-Wattribute-warning]
  529 |                         __read_overflow2_field(q_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In both cases, the compiler complains on:

memcpy(ba, &ts->ba_low, WME_BA_BMP_SIZE >> 3);

which is the legal way to copy both 'ba_low' and following 'ba_high'
members of 'struct ath_tx_status' at once (that is, issue one 8-byte
'memcpy()' for two 4-byte fields). Since the fortification logic seems
interprets this trick as an attempt to overread 4-byte 'ba_low', silence
relevant warnings by using the convenient 'struct_group()' quirk.

Suggested-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
Link: https://lore.kernel.org/r/20230620080855.396851-2-dmantipov@yandex.ru
drivers/net/wireless/ath/ath9k/mac.h
drivers/net/wireless/ath/ath9k/xmit.c

index af44b33814ddc3458c4523669c8f9ba5c1a06b2d..f03d792732da72139c24679f3ea7d4f9e55398aa 100644 (file)
@@ -115,8 +115,10 @@ struct ath_tx_status {
        u8 qid;
        u16 desc_id;
        u8 tid;
-       u32 ba_low;
-       u32 ba_high;
+       struct_group(ba,
+               u32 ba_low;
+               u32 ba_high;
+       );
        u32 evm0;
        u32 evm1;
        u32 evm2;
index 8babaaacacf59cd30bd6b29c1e5a79aad4b3de9a..4e939dcac1c9dc59682a6c30b88981d1e89a2025 100644 (file)
@@ -470,7 +470,7 @@ static void ath_tx_count_frames(struct ath_softc *sc, struct ath_buf *bf,
 
        if (isaggr) {
                seq_st = ts->ts_seqnum;
-               memcpy(ba, &ts->ba_low, WME_BA_BMP_SIZE >> 3);
+               memcpy(ba, &ts->ba, WME_BA_BMP_SIZE >> 3);
        }
 
        while (bf) {
@@ -553,7 +553,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
        if (isaggr && txok) {
                if (ts->ts_flags & ATH9K_TX_BA) {
                        seq_st = ts->ts_seqnum;
-                       memcpy(ba, &ts->ba_low, WME_BA_BMP_SIZE >> 3);
+                       memcpy(ba, &ts->ba, WME_BA_BMP_SIZE >> 3);
                } else {
                        /*
                         * AR5416 can become deaf/mute when BA