| Message ID | 1561469284-6477-1-git-send-email-shay.bar@celeno.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | HE: Consider the dynamic length of the mcs_nss and ppet fields of HE Capability IE | expand |
On Tuesday, 25 June 2019 15:28:43 CEST Shay Bar wrote: > he_capab_len is always greater than sizeof(struct ieee80211_he_capabilities) because of the dynamic mcs_nss and ppet fields. > Thus, the validity check in copy_sta_he_capab will always fail and he_capab will never be parsed. > Fix is to validate that he_capab_len is not greater than the maximum HE Capability IE size and use the actual he_capab_len to parse the he_capab. > Also, take these fields into consideration in beacon.c > > Signed-off-by: shay.bar <shay.bar@celeno.com> Your Signed-off-by name isn't equal to the author name (see "From: ") of this patch. And at least in many projects (cannot say for sure for hostapd since it seems to deal quite differently with some stuff), it is helpful to have the version number of the submission in the prefix. You can generate the patches with a version number using -v 2 (for version 2). Here an example: git format-patch -v 2 -1 This is rather helpful when you create the summary of changes for this patch since the first version. > --- > Now including the Signed-off-by :) What the heck are you using to send these mails? The header seems to suggest that you are using git-send-email to send them out but the result [1] looks like nothing which patch or git-am can parse. Is your company filtering outgoing mails to death? $ git am ~/Downloads/HE-Consider-the-dynamic-length-of-the-mcs_nss-and-ppet-fields-of-HE-Capability-IE.patch Applying: HE: Consider the dynamic length of the mcs_nss and ppet fields of HE Capability IE error: patch failed: src/ap/beacon.c:397 error: src/ap/beacon.c: patch does not apply error: patch failed: src/ap/ieee802_11_he.c:323 error: src/ap/ieee802_11_he.c: patch does not apply Patch failed at 0001 HE: Consider the dynamic length of the mcs_nss and ppet fields of HE Capability IE hint: Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". $ patch -p1 -i ~/Downloads/HE-Consider-the-dynamic-length-of-the-mcs_nss-and-ppet-fields-of-HE-Capability-IE.patch patching file src/ap/beacon.c Hunk #1 FAILED at 397. Hunk #2 FAILED at 1089. 2 out of 2 hunks FAILED -- saving rejects to file src/ap/beacon.c.rej patching file src/ap/ieee802_11_he.c Hunk #1 FAILED at 323. Hunk #2 FAILED at 333. 2 out of 2 hunks FAILED -- saving rejects to file src/ap/ieee802_11_he.c.rej [...] > @@ -333,14 +336,13 @@ u16 copy_sta_he_capab(struct hostapd_data *hapd, struct sta_info *sta, > } > > if (!sta->he_capab) { > -sta->he_capab = > -os_zalloc(sizeof(struct ieee80211_he_capabilities)); > +sta->he_capab =os_zalloc(he_capab_len); > if (!sta->he_capab) > return WLAN_STATUS_UNSPECIFIED_FAILURE; > } > > sta->flags |= WLAN_STA_HE; > -os_memset(sta->he_capab, 0, sizeof(struct ieee80211_he_capabilities)); > +os_memset(sta->he_capab, 0, he_capab_len); > os_memcpy(sta->he_capab, he_capab, he_capab_len); > sta->he_capab_len = he_capab_len; Please see https://patchwork.ozlabs.org/patch/1109462/ > ________________________________ > The information transmitted is intended only for the person or entity to which it is addressed and may contain confidential and/or privileged material. Any retransmission, dissemination, copying or other use of, or taking of any action in reliance upon this information is prohibited. If you received this in error, please contact the sender and delete the material from any computer. Nothing contained herein shall be deemed as a representation, warranty or a commitment by Celeno. No warranties are expressed or implied, including, but not limited to, any implied warranties of non-infringement, merchantability and fitness for a particular purpose. > ________________________________ This is wrong in so many ways. Kind regards, Sven [1] https://patchwork.ozlabs.org/patch/1122057/
On Wednesday, 26 June 2019 11:15:28 CEST Shay Bar wrote: > To format a patch I am using: "git format-patch origin -s". > To send it via email to hostap@lists.infradead.org, I am using: "git send-email -1". > I am using git version 1.9.1 with this .gitconfig: [...] > Can you please tell me what should I change? Start by fixing your name in the configuration. Unless your name is actually shay.bar and your mail server is something wrong by changing the "From: " to "Shay Bar <Shay.Bar@celeno.com>". In that case, get in contact with your admins to fix this problems on the mailserver. And regarding broken patch format: I would guess that it is caused by some configuration/filter on the mail server which you use to transmit messages. You could for example switch to a different server to send such mails to avoid this. But at least in theory, a vanilla smtp.office365.com should work [1]. But I don't use this server and can therefore not confirm it. Kind regards, Sven [1] https://marc.info/?l=linux-spi&m=154269987700355&w=2
On Wednesday, 26 June 2019 11:37:02 CEST Sven Eckelmann wrote: > On Wednesday, 26 June 2019 11:15:28 CEST Shay Bar wrote: > > To format a patch I am using: "git format-patch origin -s". > > To send it via email to hostap@lists.infradead.org, I am using: "git send-email -1". > > I am using git version 1.9.1 with this .gitconfig: > [...] > > Can you please tell me what should I change? [...] > And regarding broken patch format: I would guess that it is caused by some > configuration/filter on the mail server which you use to transmit messages. > You could for example switch to a different server to send such mails to avoid > this. Ah, forgot to tell you that you should also add a "from" line to your sendemail config. [sendemail] from = Shay Bar <cooluser@otherserver.com> Of course with the correct information and not these examples. Not that you start to send mails out with a faked "From:" header. And this also has the benefit that git-send-email will compare the From: line of the patch and the one in the configuration. And when they are different, it will add a secondary from line in the mail body with the original patch From: line and replace the From in the mail header with the one from .gitconfig [1]. git-am will then pick the second From: line when the patch is applied [2]. When it is done correctly, it will ensure that the author mail and the last Signed-off-by line of the mail are equal - even when submitted using a mail server for a completely different domain. Kind regards, Sven [1] https://patchwork.ozlabs.org/patch/1114908/ [2] https://w1.fi/cgit/hostap/commit/?id=29f85561894d4799a902656549a5847e4febabd3
diff --git a/src/ap/beacon.c b/src/ap/beacon.c index a51b949..98efb45 100644 --- a/src/ap/beacon.c +++ b/src/ap/beacon.c @@ -397,6 +397,8 @@ static u8 * hostapd_gen_probe_resp(struct hostapd_data *hapd, #ifdef CONFIG_IEEE80211AX if (hapd->iconf->ieee80211ax) { buflen += 3 + sizeof(struct ieee80211_he_capabilities) + +HE_MAX_MCS_CAPAB_SIZE + +HE_MAX_PPET_CAPAB_SIZE + 3 + sizeof(struct ieee80211_he_operation) + 3 + sizeof(struct ieee80211_he_mu_edca_parameter_set) + 3 + sizeof(struct ieee80211_spatial_reuse); @@ -1089,6 +1091,8 @@ int ieee802_11_build_ap_params(struct hostapd_data *hapd, #ifdef CONFIG_IEEE80211AX if (hapd->iconf->ieee80211ax) { tail_len += 3 + sizeof(struct ieee80211_he_capabilities) + +HE_MAX_MCS_CAPAB_SIZE + +HE_MAX_PPET_CAPAB_SIZE + 3 + sizeof(struct ieee80211_he_operation) + 3 + sizeof(struct ieee80211_he_mu_edca_parameter_set) + 3 + sizeof(struct ieee80211_spatial_reuse); diff --git a/src/ap/ieee802_11_he.c b/src/ap/ieee802_11_he.c index a51f3fc..a7a74f0 100644 --- a/src/ap/ieee802_11_he.c +++ b/src/ap/ieee802_11_he.c @@ -323,9 +323,12 @@ u16 copy_sta_he_capab(struct hostapd_data *hapd, struct sta_info *sta, enum ieee80211_op_mode opmode, const u8 *he_capab, size_t he_capab_len) { +size_t he_capab_max_len = sizeof(struct ieee80211_he_capabilities) + +HE_MAX_MCS_CAPAB_SIZE + +HE_MAX_PPET_CAPAB_SIZE; if (!he_capab || !hapd->iconf->ieee80211ax || !check_valid_he_mcs(hapd, he_capab, opmode) || - he_capab_len > sizeof(struct ieee80211_he_capabilities)) { + he_capab_len > he_capab_max_len) { sta->flags &= ~WLAN_STA_HE; os_free(sta->he_capab); sta->he_capab = NULL; @@ -333,14 +336,13 @@ u16 copy_sta_he_capab(struct hostapd_data *hapd, struct sta_info *sta, } if (!sta->he_capab) { -sta->he_capab = -os_zalloc(sizeof(struct ieee80211_he_capabilities)); +sta->he_capab =os_zalloc(he_capab_len); if (!sta->he_capab) return WLAN_STATUS_UNSPECIFIED_FAILURE; } sta->flags |= WLAN_STA_HE; -os_memset(sta->he_capab, 0, sizeof(struct ieee80211_he_capabilities)); +os_memset(sta->he_capab, 0, he_capab_len); os_memcpy(sta->he_capab, he_capab, he_capab_len); sta->he_capab_len = he_capab_len;
he_capab_len is always greater than sizeof(struct ieee80211_he_capabilities) because of the dynamic mcs_nss and ppet fields. Thus, the validity check in copy_sta_he_capab will always fail and he_capab will never be parsed. Fix is to validate that he_capab_len is not greater than the maximum HE Capability IE size and use the actual he_capab_len to parse the he_capab. Also, take these fields into consideration in beacon.c Signed-off-by: shay.bar <shay.bar@celeno.com> --- Now including the Signed-off-by :) src/ap/beacon.c | 4 ++++ src/ap/ieee802_11_he.c | 10 ++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) -- 1.9.1