--- src/eap-mschapv2.h | 1 - src/eap-pwd.c | 1 - src/ie.c | 1 - 3 files changed, 3 deletions(-) diff --git a/src/eap-mschapv2.h b/src/eap-mschapv2.h index 6b090969..d1f4fd53 100644 --- a/src/eap-mschapv2.h +++ b/src/eap-mschapv2.h @@ -28,7 +28,6 @@ #include <stdint.h> #include <stdbool.h> - bool mschapv2_get_asymmetric_start_key(const uint8_t master_key[static 16], uint8_t *session_key, size_t session_len, bool server, bool send); diff --git a/src/eap-pwd.c b/src/eap-pwd.c index 74d0997f..a77f21b9 100644 --- a/src/eap-pwd.c +++ b/src/eap-pwd.c @@ -435,7 +435,6 @@ error: eap_method_error(eap); } - static void eap_pwd_handle_confirm(struct eap_state *eap, const uint8_t *pkt, size_t len) { diff --git a/src/ie.c b/src/ie.c index 9b1127ce..f120c89e 100644 --- a/src/ie.c +++ b/src/ie.c @@ -2406,7 +2406,6 @@ int ie_parse_neighbor_report(struct ie_tlv_iter *iter, return 0; } - int ie_parse_roaming_consortium(struct ie_tlv_iter *iter, size_t *num_anqp_out, const uint8_t **oi1_out, size_t *oi1_len_out, const uint8_t **oi2_out, size_t *oi2_len_out, -- 2.20.1

Hi Andrew, On 3/18/20 9:45 AM, Andrew Zaborowski wrote: This part shouldn't be necessary? io_closed clears out the disconnect_handler and disconnect_destroy callbacks... Rest looks good. Let me know if I should just edit out the change above. Regards, -Denis

Fix a potential leak when we need to drop an existing watch because it's being replaced with a new one. --- src/frame-xchg.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/frame-xchg.c b/src/frame-xchg.c index 3719ed6c..68bd6168 100644 --- a/src/frame-xchg.c +++ b/src/frame-xchg.c @@ -524,6 +524,7 @@ static bool frame_watch_check_duplicate(void *data, void *user_data) } /* Drop the existing watch as a duplicate of the new one */ + frame_watch_free(&watch->super); return true; } -- 2.20.1

--- src/frame-xchg.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/frame-xchg.c b/src/frame-xchg.c index 68bd6168..7b45ebb6 100644 --- a/src/frame-xchg.c +++ b/src/frame-xchg.c @@ -658,7 +658,8 @@ struct frame_watch_handler_check_info { static bool frame_watch_item_remove_by_handler(void *data, void *user_data) { - struct frame_watch *watch = data; + struct frame_watch *watch = + l_container_of(data, struct frame_watch, super); struct frame_watch_handler_check_info *info = user_data; if (watch->super.notify != info->handler || -- 2.20.1

Make sure a frame callback is free to call frame_xchg_stop without causing a crash. Frame callback here means the one that gets called if our tx frame was ACKed and triggered a respone frame that matched one of the provided prefixes, within the given time. All in all a frame callback is allowed to call either frame_xchg_stop or frame_xchg_startv or neither. Same applies to the final callback (called when no matching responses received). --- src/frame-xchg.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/frame-xchg.c b/src/frame-xchg.c index 7b66cb6d..dd6d63ad 100644 --- a/src/frame-xchg.c +++ b/src/frame-xchg.c @@ -108,6 +108,7 @@ struct frame_xchg_data { unsigned int retry_interval; unsigned int resp_timeout; bool in_frame_cb : 1; + bool stale : 1; }; struct frame_xchg_watch_data { @@ -725,6 +726,7 @@ static void frame_xchg_wait_cancel(struct frame_xchg_data *fx) static void frame_xchg_reset(struct frame_xchg_data *fx) { fx->in_frame_cb = false; + fx->stale = false; frame_xchg_wait_cancel(fx); @@ -972,7 +974,7 @@ static void frame_xchg_resp_cb(const struct mmpdu_header *mpdu, fx->in_frame_cb = false; - if (done) { + if (done || fx->stale) { fx->cb = NULL; frame_xchg_done(fx, 0); return; @@ -1117,6 +1119,11 @@ void frame_xchg_stop(uint64_t wdev_id) if (!fx) return; + if (fx->in_frame_cb) { + fx->stale = true; + return; + } + frame_xchg_reset(fx); l_free(fx); } -- 2.20.1

In some cases a P2P peer will ACK our frame but not reply on the first attempt, and other implementations seem to handle this by going back to retransmitting the frame at a high rate until it gets ACKed again, at which point they will again give the peer a longer time to tx the response frame. Implement the same logic here by adding a retries_on_ack parameter that takes the number of additional times we want to restart the normal retransmit counter after we received no response frame on the first attempt. So passing 0 maintains the current behaviour, 1 for 1 extra attempt, etc. In effect we may retransmit a frame about 15 * (retry_on_ack + 1) * <in-kernel retransmit limit> times. The kernel/driver retransmits a frame a number of times if there's no ACK (I've seen about 20 normally) at a high frequency, if that fails we retry the whole process 15 times inside frame-xchg.c and if we still get no ACK at any point, we give up. If we do get an ACK, we wait for a response frame and if we don't get that we will optionally reset the retry counter and restart the whole thing retry_on_ack times. --- src/frame-xchg.c | 61 +++++++++++++++++++++++++++++++++--------------- src/frame-xchg.h | 4 ++-- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/src/frame-xchg.c b/src/frame-xchg.c index a7565a24..a4b6b740 100644 --- a/src/frame-xchg.c +++ b/src/frame-xchg.c @@ -107,6 +107,7 @@ struct frame_xchg_data { unsigned int retry_cnt; unsigned int retry_interval; unsigned int resp_timeout; + unsigned int retries_on_ack; bool in_frame_cb : 1; bool stale : 1; }; @@ -722,12 +723,16 @@ static bool frame_watch_remove_by_handler(uint64_t wdev_id, uint32_t group_id, if (!group) return false; + l_error(" ** settign grp %i elems that match to nop", group->id);//// return l_queue_foreach_remove(group->watches.items, frame_watch_item_remove_by_handler, &handler_info) > 0; } static void frame_xchg_tx_retry(struct frame_xchg_data *fx); +static bool frame_xchg_resp_handle(const struct mmpdu_header *mpdu, + const void *body, size_t body_len, + int rssi, void *user_data); static void frame_xchg_resp_cb(const struct mmpdu_header *mpdu, const void *body, size_t body_len, int rssi, void *user_data); @@ -807,12 +812,19 @@ static void frame_xchg_timeout_cb(struct l_timeout *timeout, frame_xchg_tx_retry(fx); } -static void frame_xchg_resp_timeout_cb(struct l_timeout *timeout, +static void frame_xchg_listen_end_cb(struct l_timeout *timeout, void *user_data) { struct frame_xchg_data *fx = user_data; - frame_xchg_done(fx, 0); + if (!fx->retries_on_ack--) { + frame_xchg_done(fx, 0); + return; + } + + l_timeout_remove(fx->timeout); + fx->retry_cnt = 0; + frame_xchg_tx_retry(fx); } static void frame_xchg_tx_status(struct frame_xchg_data *fx, bool acked) @@ -852,17 +864,19 @@ static void frame_xchg_tx_status(struct frame_xchg_data *fx, bool acked) fx->have_cookie = false; l_debug("Processing an early frame"); - frame_xchg_resp_cb(fx->early_frame.mpdu, fx->early_frame.body, - fx->early_frame.body_len, - fx->early_frame.rssi, fx); - frame_xchg_done(fx, 0); + if (frame_xchg_resp_handle(fx->early_frame.mpdu, fx->early_frame.body, + fx->early_frame.body_len, + fx->early_frame.rssi, fx)) + return; + + frame_xchg_listen_end_cb(NULL, fx); return; } /* Txed frame ACKed, listen for response frames */ fx->timeout = l_timeout_create_ms(fx->resp_timeout, - frame_xchg_resp_timeout_cb, fx, + frame_xchg_listen_end_cb, fx, frame_xchg_timeout_destroy); } @@ -948,9 +962,9 @@ static void frame_xchg_tx_retry(struct frame_xchg_data *fx) fx->retry_cnt++; } -static void frame_xchg_resp_cb(const struct mmpdu_header *mpdu, - const void *body, size_t body_len, - int rssi, void *user_data) +static bool frame_xchg_resp_handle(const struct mmpdu_header *mpdu, + const void *body, size_t body_len, + int rssi, void *user_data) { struct frame_xchg_data *fx = user_data; const struct l_queue_entry *entry; @@ -959,10 +973,10 @@ static void frame_xchg_resp_cb(const struct mmpdu_header *mpdu, l_debug(""); if (memcmp(mpdu->address_1, fx->tx_mpdu->address_2, 6)) - return; + return false; if (memcmp(mpdu->address_2, fx->tx_mpdu->address_1, 6)) - return; + return false; /* * Is the received frame's BSSID same as the transmitted frame's @@ -972,7 +986,7 @@ static void frame_xchg_resp_cb(const struct mmpdu_header *mpdu, */ if (memcmp(mpdu->address_3, fx->tx_mpdu->address_3, 6) && !util_mem_is_zero(mpdu->address_3, 6)) - return; + return false; for (entry = l_queue_get_entries(fx->rx_watches); entry; entry = entry->next) { @@ -996,18 +1010,18 @@ static void frame_xchg_resp_cb(const struct mmpdu_header *mpdu, * to just exit without touching anything. */ if (!fx->in_frame_cb) - return; + return true; fx->in_frame_cb = false; if (done || fx->stale) { fx->cb = NULL; frame_xchg_done(fx, 0); - return; + return true; } } - return; + return false; early_frame: /* @@ -1018,13 +1032,21 @@ early_frame: * Save the response frame to be processed in the Tx done callback. */ if (fx->early_frame.mpdu) - return; + return false; hdr_len = (const uint8_t *) body - (const uint8_t *) mpdu; fx->early_frame.mpdu = l_memdup(mpdu, body_len + hdr_len); fx->early_frame.body = (const uint8_t *) fx->early_frame.mpdu + hdr_len; fx->early_frame.body_len = body_len; fx->early_frame.rssi = rssi; + return false; +} + +static void frame_xchg_resp_cb(const struct mmpdu_header *mpdu, + const void *body, size_t body_len, + int rssi, void *user_data) +{ + frame_xchg_resp_handle(mpdu, body, body_len, rssi, user_data); } static bool frame_xchg_match(const void *a, const void *b) @@ -1052,8 +1074,8 @@ static bool frame_xchg_match(const void *a, const void *b) */ void frame_xchg_startv(uint64_t wdev_id, struct iovec *frame, uint32_t freq, unsigned int retry_interval, unsigned int resp_timeout, - uint32_t group_id, frame_xchg_cb_t cb, void *user_data, - va_list resp_args) + unsigned int retries_on_ack, uint32_t group_id, + frame_xchg_cb_t cb, void *user_data, va_list resp_args) { struct frame_xchg_data *fx; size_t frame_len; @@ -1097,6 +1119,7 @@ void frame_xchg_startv(uint64_t wdev_id, struct iovec *frame, uint32_t freq, fx->freq = freq; fx->retry_interval = retry_interval; fx->resp_timeout = resp_timeout; + fx->retries_on_ack = retries_on_ack; fx->cb = cb; fx->user_data = user_data; fx->group_id = group_id; diff --git a/src/frame-xchg.h b/src/frame-xchg.h index 1855492c..93d528ae 100644 --- a/src/frame-xchg.h +++ b/src/frame-xchg.h @@ -45,6 +45,6 @@ bool frame_watch_wdev_remove(uint64_t wdev_id); void frame_xchg_startv(uint64_t wdev_id, struct iovec *frame, uint32_t freq, unsigned int retry_interval, unsigned int resp_timeout, - uint32_t group_id, frame_xchg_cb_t cb, void *user_data, - va_list resp_args); + unsigned int retries_on_ack, uint32_t group_id, + frame_xchg_cb_t cb, void *user_data, va_list resp_args); void frame_xchg_stop(uint64_t wdev_id); -- 2.20.1

Hi Denis, On Wed, 18 Mar 2020 at 23:09, Denis Kenzior <denkenz(a)gmail.com&gt; wrote: Oops, a git add screwup. Ok, but this is the same standard thing you use in a for loop ;) Best regards

Work around a parse error in GO Negotiation with some P2P devices. --- src/p2putil.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/p2putil.c b/src/p2putil.c index 02c02b4d..58a75a11 100644 --- a/src/p2putil.c +++ b/src/p2putil.c @@ -193,7 +193,12 @@ static bool extract_p2p_channel_list(const uint8_t *attr, size_t len, { struct p2p_channel_list_attr *out = data; - if (len < 6) + /* + * Some devices reply with an empty Channel Entry List inside the + * Channel List attribute of a GO Negotiation Response (status 1), + * so tolerate a length of 3. + */ + if (len < 3) return false; out->country[0] = *attr++; -- 2.20.1