sys/net/application_layer/gcoap: Add a mutex to avoid race condition

RIOT-OS · Apr 24, 2024 · d0c6ef5 · d0c6ef5
1 parent 4d8c173
commit d0c6ef5
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 11 deletions.
diff --git a/sys/include/net/gcoap/forward_proxy.h b/sys/include/net/gcoap/forward_proxy.h
@@ -97,6 +97,21 @@ void gcoap_forward_proxy_find_req_memo(gcoap_request_memo_t **memo_ptr,
                                        coap_pkt_t *src_pdu,
                                        const sock_udp_ep_t *remote);
 
+/**
+ * @brief   This function allocates client endpoint
+ *
+ * @param[in] ep    client endpoint
+ * @return  client_ep_t
+ */
+client_ep_t *gcoap_forward_proxy_allocate_client_ep(const sock_udp_ep_t *ep);
+
+/**
+ * @brief   This function frees client endpoint
+ *
+ * @param[in] cep    client endpoint
+ */
+void gcoap_forward_proxy_free_client_ep(client_ep_t *cep);
+
 #ifdef __cplusplus
 }
 #endif

diff --git a/sys/net/application_layer/gcoap/forward_proxy.c b/sys/net/application_layer/gcoap/forward_proxy.c
@@ -35,7 +35,11 @@
 #define CLIENT_EP_FLAGS_ETAG_LEN_MASK   0x0f
 #define CLIENT_EP_FLAGS_ETAG_LEN_POS    0U
 
+#define GCOAP_FP_PROXY_LOCK_TIMEOUT_MS  1000
+
 extern kernel_pid_t _forward_proxy_pid;
+static mutex_t _fp_mutex = MUTEX_INIT;
+
 extern uint16_t gcoap_next_msg_id(void);
 extern void gcoap_forward_proxy_post_event(void *arg);
 
@@ -83,7 +87,7 @@ void gcoap_forward_proxy_init(void)
     }
 }
 
-static client_ep_t *_allocate_client_ep(const sock_udp_ep_t *ep)
+client_ep_t *gcoap_forward_proxy_allocate_client_ep(const sock_udp_ep_t *ep)
 {
     client_ep_t *cep;
     for (cep = _client_eps;
@@ -99,10 +103,13 @@ static client_ep_t *_allocate_client_ep(const sock_udp_ep_t *ep)
     return NULL;
 }
 
-static void _free_client_ep(client_ep_t *cep)
+void gcoap_forward_proxy_free_client_ep(client_ep_t *cep)
 {
-    ztimer_remove(ZTIMER_MSEC, &cep->empty_ack_timer);
-    memset(cep, 0, sizeof(*cep));
+    if (!ztimer_mutex_lock_timeout(ZTIMER_MSEC, &_fp_mutex, GCOAP_FP_PROXY_LOCK_TIMEOUT_MS)) {
+        ztimer_remove(ZTIMER_MSEC, &cep->empty_ack_timer);
+        memset(cep, 0, sizeof(*cep));
+        mutex_unlock(&_fp_mutex);
+    }
 }
 
 static int _request_matcher_forward_proxy(gcoap_listener_t *listener,
@@ -298,7 +305,7 @@ void _forward_resp_handler(const gcoap_request_memo_t *memo,
     _set_response_type(pdu, _cep_get_response_type(cep));
     /* don't use buf_len here, in case the above `gcoap_resp_init`s changed `pdu` */
     _dispatch_msg(pdu->hdr, coap_get_total_len(pdu), &cep->ep);
-    _free_client_ep(cep);
+    gcoap_forward_proxy_free_client_ep(cep);
 }
 
 static int _gcoap_forward_proxy_add_uri_path(coap_pkt_t *pkt,
@@ -401,7 +408,7 @@ static int _gcoap_forward_proxy_via_coap(coap_pkt_t *client_pkt,
     gcoap_forward_proxy_find_req_memo(&memo, client_pkt, &client_ep->server_ep);
     if (memo) {
         DEBUG("gcoap_forward_proxy: request already exists, ignore!\n");
-        _free_client_ep(client_ep);
+        gcoap_forward_proxy_free_client_ep(client_ep);
         return 0;
     }
 
@@ -456,7 +463,7 @@ int gcoap_forward_proxy_request_process(coap_pkt_t *pkt,
     uri_parser_result_t urip;
     ssize_t optlen = 0;
 
-    client_ep_t *cep = _allocate_client_ep(client);
+    client_ep_t *cep = gcoap_forward_proxy_allocate_client_ep(client);
 
     if (!cep) {
         return -ENOMEM;
@@ -472,15 +479,15 @@ int gcoap_forward_proxy_request_process(coap_pkt_t *pkt,
 
     if (optlen < 0) {
         /* -ENOENT, -EINVAL */
-        _free_client_ep(cep);
+        gcoap_forward_proxy_free_client_ep(cep);
         return optlen;
     }
 
     int ures = uri_parser_process(&urip, (const char *) uri, optlen);
 
     /* cannot parse Proxy-URI option, or URI is relative */
     if (ures || (!uri_parser_is_absolute((const char *) uri, optlen))) {
-        _free_client_ep(cep);
+        gcoap_forward_proxy_free_client_ep(cep);
         return -EINVAL;
     }
 
@@ -489,13 +496,13 @@ int gcoap_forward_proxy_request_process(coap_pkt_t *pkt,
         !strncmp("coaps", urip.scheme, urip.scheme_len)) {
         int res = _gcoap_forward_proxy_via_coap(pkt, cep, &urip);
         if (res < 0) {
-            _free_client_ep(cep);
+            gcoap_forward_proxy_free_client_ep(cep);
             return -EINVAL;
         }
     }
     /* no other scheme supported for now */
     else {
-        _free_client_ep(cep);
+        gcoap_forward_proxy_free_client_ep(cep);
         return -EPERM;
     }
 

diff --git a/sys/net/application_layer/gcoap/forward_proxy_thread.c b/sys/net/application_layer/gcoap/forward_proxy_thread.c
@@ -46,6 +46,7 @@ static void *_forward_proxy_thread_start(void *arg)
                                 &cep->server_ep, _forward_resp_handler, cep,
                                 GCOAP_SOCKET_TYPE_UNDEF) < 0) {
                     DEBUG_PUTS("_forward_proxy_thread_start: gcoap_req_send failed\n");
+                    gcoap_forward_proxy_free_client_ep(cep);
                 }
                 break;
             }