Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Packet injection bugs for control frames #162

Open
cloudswei opened this issue Jul 23, 2020 · 4 comments
Open

Packet injection bugs for control frames #162

cloudswei opened this issue Jul 23, 2020 · 4 comments

Comments

@cloudswei
Copy link
Contributor

cloudswei commented Jul 23, 2020

There are two issues when injecting control frames (RTS/CTS) to an ath9k-htc interface in monitor mode:

  1. CTS packets are truncated and malformed
  2. The unicast control frames are always re-transmitted unnecessarily to wait for an ACK

Reproduces:

Inject a CTS frame like below:

IEEE 802.11 Clear-to-send, Flags: ........C
Type/Subtype: Clear-to-send (0x001c)
Frame Control Field: 0xc400
.... ..00 = Version: 0
.... 01.. = Type: Control frame (1)
1100 .... = Subtype: 12
Flags: 0x00
.000 0000 1101 0001 = Duration: 209 microseconds
Receiver address: 00:11:22:33:44:55 (00:11:22:33:44:55)

The captured frame over-the-air becomes:

IEEE 802.11 Clear-to-send, Flags: .....R..C
Type/Subtype: Clear-to-send (0x001c)
Frame Control Field: 0xc408
.... ..00 = Version: 0
.... 01.. = Type: Control frame (1)
1100 .... = Subtype: 12
Flags: 0x08
.000 0000 1101 0001 = Duration: 209 microseconds
[Expert Info (Error/Malformed): Malformed Packet (Exception occurred)]

The frame is malformed because the receiver address (00:11:22:33:44:55) is truncated to become 00:11:22:33, two ending bytes (44:55) are missing. And this CTS frame is re-transmitted by multiple times (R flag is set).

The reason is current ath9k_htc FW doesn't have TX handler for control frames, so all non-data frames are processed by management frame handler ath_tgt_send_mgt(). Function ath_tgt_send_mgt() expects all management frames to be in 3-address format and therefore the MAC header size should be 24 bytes (4-bye aligned), and unicast management frames need to be retransmitted when ACK is missing. Those assumptions are also applied to control frames, which results in truncating 2 bytes from CTS packet's receiver address and unnecessary retransmissions.

The fix is straightforward: remove the unnecessary alignment/truncation on MAC headers and apply NOACK flag to control frame's TX descriptor. After applying the patch below, the two issues can be fixed and the control-frame packet injection is working as expected.

diff --git a/target_firmware/wlan/if_owl.c b/target_firmware/wlan/if_owl.c
index 6dda78c..e522297 100755
--- a/target_firmware/wlan/if_owl.c
+++ b/target_firmware/wlan/if_owl.c
@@ -1108,7 +1108,7 @@ ath_tgt_send_mgt(struct ath_softc_tgt *sc,adf_nbuf_t hdr_buf, adf_nbuf_t skb,
        hdrlen = ieee80211_anyhdrsize(wh);
        pktlen = len;
        keyix = HAL_TXKEYIX_INVALID;
-       pktlen -= (hdrlen & 3);
+/*     pktlen -= (hdrlen & 3);*/
        pktlen += IEEE80211_CRC_LEN;
 
        if (iswep)
@@ -1142,6 +1142,8 @@ ath_tgt_send_mgt(struct ath_softc_tgt *sc,adf_nbuf_t hdr_buf, adf_nbuf_t skb,
                        atype = HAL_PKT_TYPE_NORMAL;
 
                break;
+       case IEEE80211_FC0_TYPE_CTL:
+               flags |= HAL_TXDESC_NOACK;
        default:
                atype = HAL_PKT_TYPE_NORMAL;
                break;
@erikarn
Copy link
Collaborator

erikarn commented Jul 23, 2020 via email

cloudswei added a commit to cloudswei/open-ath9k-htc-firmware that referenced this issue Jul 23, 2020
Fix issue: Packet injection bugs for control frames qca#162

Signed-off-by: Clouds Wei <cloudswei@gmail.com>
@erikarn
Copy link
Collaborator

erikarn commented Jul 23, 2020

hm, i'm not sure i get why the alignment thing isn't breaking on other mgmt traffic. shouldn't we be aligning the control frames that you're injecting via monitor mode?

@erikarn
Copy link
Collaborator

erikarn commented Jul 23, 2020

(the NOACK on control frames is good though!)

@cloudswei
Copy link
Contributor Author

Since for a management frame, ieee80211_anyhdrsize() returns sizeof(struct ieee80211_frame), which is 2+2+6+6+6+2 = 24. So pktlen -= (hdrlen & 3) is equivalent to pktlen -= (24 & 3) or pktlen -= 0, actually doing nothing.

struct ieee80211_frame {
a_uint8_t i_fc[2];
a_uint8_t i_dur[2];
a_uint8_t i_addr1[IEEE80211_ADDR_LEN];
a_uint8_t i_addr2[IEEE80211_ADDR_LEN];
a_uint8_t i_addr3[IEEE80211_ADDR_LEN];
a_uint8_t i_seq[2];
/* possibly followed by addr4[IEEE80211_ADDR_LEN]; /
/
see below */
} adf_os_packed;

For monitor mode (maybe for normal operational mode also), we should not change anything on the frames passed from host.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants