Hi Brian, > Hi Isak, > > Have you run this patch through any runtime analysis (like valgrind > etc) to ensure it has introduced no memory leaks? > > I am particularily concerned with any l_timeout_remove() calls that don't immediately set the freed timer pointer to NULL, and any new > l_timeout_create() calls that are not preceded with a check that there is not already a valid pointer occupying the seg_timeout or msg_timeout members. > I tested it with valgrind and found no memory leaks. However, it is perhaps anyway a better practice to use l_timeout_modify() ? If so, I will update the patch. > On Thu, 2022-10-06 at 16:59 +0200, Isak Westin wrote: > > When a SAR transmission has been completed or canceled, the recipent > > should store the block authentication values for at least 10 seconds > > and ignore new segments with the same values during this period. See > > MshPRFv1.0.1 section 3.5.3.4. > > --- > > mesh/net.c | 30 ++++++++++++++++++++++++++++-- > > 1 file changed, 28 insertions(+), 2 deletions(-) > > > > diff --git a/mesh/net.c b/mesh/net.c > > index 379a6e250..e95ae5114 100644 > > --- a/mesh/net.c > > +++ b/mesh/net.c > > @@ -46,6 +46,7 @@ > > > > #define SEG_TO 2 > > #define MSG_TO 60 > > +#define SAR_DEL 10 > > > > #define DEFAULT_TRANSMIT_COUNT 1 > > #define DEFAULT_TRANSMIT_INTERVAL 100 > > @@ -166,6 +167,7 @@ struct mesh_sar { > > bool segmented; > > bool frnd; > > bool frnd_cred; > > + bool delete; > > uint8_t ttl; > > uint8_t last_seg; > > uint8_t key_aid; > > @@ -1493,13 +1495,27 @@ static void inseg_to(struct l_timeout > > *seg_timeout, void *user_data) static void inmsg_to(struct l_timeout > > *msg_timeout, void *user_data) { > > struct mesh_net *net = user_data; > > - struct mesh_sar *sar = l_queue_remove_if(net->sar_in, > > + struct mesh_sar *sar = l_queue_find(net->sar_in, > > match_msg_timeout, msg_timeout); > > > > l_timeout_remove(msg_timeout); > > if (!sar) > > return; > > > > + if (!sar->delete) { > > + /* > > + * Incomplete timer expired, cancel SAR and start > > + * delete timer > > + */ > > + sar->delete = true; > > + l_timeout_remove(sar->seg_timeout); > > + sar->seg_timeout = NULL; > > + sar->msg_timeout = l_timeout_create(SAR_DEL, > > + inmsg_to, net, NULL); > > + return; > > + } > > + > > + l_queue_remove(net->sar_in, sar); > > sar->msg_timeout = NULL; > > mesh_sar_free(sar); > > } > > @@ -1956,7 +1972,9 @@ static bool seg_rxed(struct mesh_net *net, bool > > frnd, uint32_t iv_index, > > /* Re-Send ACK for full msg */ > > send_net_ack(net, sar_in, expected); > > return true; > > - } > > + } else if (sar_in->delete) > > + /* Ignore canceled */ > > + return false; > > } else { > > uint16_t len = MAX_SEG_TO_LEN(segN); > > > > @@ -1996,6 +2014,9 @@ static bool seg_rxed(struct mesh_net *net, bool > > frnd, uint32_t iv_index, > > sar_in->len = segN * MAX_SEG_LEN + size; > > > > if (sar_in->flags == expected) { > > + /* Remove message incomplete timer */ > > + l_timeout_remove(sar_in->msg_timeout); > > + > > /* Got it all */ > > send_net_ack(net, sar_in, expected); > > > > @@ -2006,6 +2027,11 @@ static bool seg_rxed(struct mesh_net *net, bool > > frnd, uint32_t iv_index, > > /* Kill Inter-Seg timeout */ > > l_timeout_remove(sar_in->seg_timeout); > > sar_in->seg_timeout = NULL; > > + > > + /* Start delete timer */ > > + sar_in->delete = true; > > + sar_in->msg_timeout = l_timeout_create(SAR_DEL, > > + inmsg_to, net, NULL); > > return true; > > } > > > >