From ca83b4df945638a7e53bbade379de4f8cd6622fe Mon Sep 17 00:00:00 2001 From: edvard Date: Mon, 9 Mar 2026 20:56:17 +0100 Subject: [PATCH 1/3] pbuf: fix use after free possibility and fast return After freeing packet the code uses packet->next Its more of an issue when task_woken is NULL meaning we are not in ISR Also added fast bail out with return to jump out of while loop --- src/interfaces/csp_if_can_pbuf.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/interfaces/csp_if_can_pbuf.c b/src/interfaces/csp_if_can_pbuf.c index faca94e1e..6039848a1 100644 --- a/src/interfaces/csp_if_can_pbuf.c +++ b/src/interfaces/csp_if_can_pbuf.c @@ -19,14 +19,16 @@ void csp_can_pbuf_free(csp_can_interface_data_t * ifdata, csp_packet_t * buffer, while (packet) { + csp_packet_t * next = packet->next; + /* Perform cleanup in used pbufs */ if (packet == buffer) { /* Erase from list prev->next = next */ if (prev) { - prev->next = packet->next; + prev->next = next; } else { - ifdata->pbufs = packet->next; + ifdata->pbufs = next; } if (buf_free) { @@ -36,11 +38,11 @@ void csp_can_pbuf_free(csp_can_interface_data_t * ifdata, csp_packet_t * buffer, csp_buffer_free_isr(packet); } } - + return; } prev = packet; - packet = packet->next; + packet = next; } } From c71cc458720c95e3f9fe1c4c435dd9af3847e476 Mon Sep 17 00:00:00 2001 From: edvard Date: Mon, 9 Mar 2026 21:15:09 +0100 Subject: [PATCH 2/3] pbufs: eth & can remove use after free possiblity After freeing packet the code uses packet->next Its more of an issue when task_woken is NULL meaning we are not in ISR Skip updating prev if packet is freed --- src/interfaces/csp_if_can_pbuf.c | 11 ++++++++--- src/interfaces/csp_if_eth_pbuf.c | 20 ++++++++++++++------ 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/interfaces/csp_if_can_pbuf.c b/src/interfaces/csp_if_can_pbuf.c index 6039848a1..26dc8ccae 100644 --- a/src/interfaces/csp_if_can_pbuf.c +++ b/src/interfaces/csp_if_can_pbuf.c @@ -75,14 +75,16 @@ void csp_can_pbuf_cleanup(csp_can_interface_data_t * ifdata, int * task_woken) { while (packet) { + csp_packet_t * next = packet->next; + /* Perform cleanup in used pbufs */ if (now - packet->last_used > PBUF_TIMEOUT_MS) { /* Erase from list prev->next = next */ if (prev) { - prev->next = packet->next; + prev->next = next; } else { - ifdata->pbufs = packet->next; + ifdata->pbufs = next; } if (task_woken == NULL) { @@ -91,10 +93,13 @@ void csp_can_pbuf_cleanup(csp_can_interface_data_t * ifdata, int * task_woken) { csp_buffer_free_isr(packet); } + packet = next; + continue; + } prev = packet; - packet = packet->next; + packet = next; } } diff --git a/src/interfaces/csp_if_eth_pbuf.c b/src/interfaces/csp_if_eth_pbuf.c index 3d7473645..75336c25e 100644 --- a/src/interfaces/csp_if_eth_pbuf.c +++ b/src/interfaces/csp_if_eth_pbuf.c @@ -16,14 +16,16 @@ void csp_eth_pbuf_free(csp_eth_interface_data_t * ifdata, csp_packet_t * buffer, while (packet) { + csp_packet_t * next = packet->next; + /* Perform cleanup in used pbufs */ if (packet == buffer) { /* Erase from list prev->next = next */ if (prev) { - prev->next = packet->next; + prev->next = next; } else { - ifdata->pbufs = packet->next; + ifdata->pbufs = next; } if (buf_free) { @@ -33,10 +35,11 @@ void csp_eth_pbuf_free(csp_eth_interface_data_t * ifdata, csp_packet_t * buffer, csp_buffer_free_isr(packet); } } + return; } prev = packet; - packet = packet->next; + packet = next; } } @@ -68,14 +71,16 @@ void csp_eth_pbuf_cleanup(csp_eth_interface_data_t * ifdata, uint32_t now, int * while (packet) { + csp_packet_t * next = packet->next; + /* Perform cleanup in used pbufs */ if ((now - packet->last_used) > PBUF_TIMEOUT_MS) { /* Erase from list prev->next = next */ if (prev) { - prev->next = packet->next; + prev->next = next; } else { - ifdata->pbufs = packet->next; + ifdata->pbufs = next; } if (task_woken == NULL) { @@ -84,10 +89,13 @@ void csp_eth_pbuf_cleanup(csp_eth_interface_data_t * ifdata, uint32_t now, int * csp_buffer_free_isr(packet); } + packet = next; + continue; + } prev = packet; - packet = packet->next; + packet = next; } } From 43d44e1be0c8546e0a714e000b8c363af28adeff Mon Sep 17 00:00:00 2001 From: edvard Date: Tue, 10 Mar 2026 08:48:23 +0100 Subject: [PATCH 3/3] pbuf: pbuf free just use packet->next The new return makes sure that it will not deref the freed packet next iter. --- src/interfaces/csp_if_can_pbuf.c | 8 +++----- src/interfaces/csp_if_eth_pbuf.c | 8 +++----- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/interfaces/csp_if_can_pbuf.c b/src/interfaces/csp_if_can_pbuf.c index 26dc8ccae..24c3360b8 100644 --- a/src/interfaces/csp_if_can_pbuf.c +++ b/src/interfaces/csp_if_can_pbuf.c @@ -19,16 +19,14 @@ void csp_can_pbuf_free(csp_can_interface_data_t * ifdata, csp_packet_t * buffer, while (packet) { - csp_packet_t * next = packet->next; - /* Perform cleanup in used pbufs */ if (packet == buffer) { /* Erase from list prev->next = next */ if (prev) { - prev->next = next; + prev->next = packet->next; } else { - ifdata->pbufs = next; + ifdata->pbufs = packet->next; } if (buf_free) { @@ -42,7 +40,7 @@ void csp_can_pbuf_free(csp_can_interface_data_t * ifdata, csp_packet_t * buffer, } prev = packet; - packet = next; + packet = packet->next; } } diff --git a/src/interfaces/csp_if_eth_pbuf.c b/src/interfaces/csp_if_eth_pbuf.c index 75336c25e..b212ab08c 100644 --- a/src/interfaces/csp_if_eth_pbuf.c +++ b/src/interfaces/csp_if_eth_pbuf.c @@ -16,16 +16,14 @@ void csp_eth_pbuf_free(csp_eth_interface_data_t * ifdata, csp_packet_t * buffer, while (packet) { - csp_packet_t * next = packet->next; - /* Perform cleanup in used pbufs */ if (packet == buffer) { /* Erase from list prev->next = next */ if (prev) { - prev->next = next; + prev->next = packet->next; } else { - ifdata->pbufs = next; + ifdata->pbufs = packet->next; } if (buf_free) { @@ -39,7 +37,7 @@ void csp_eth_pbuf_free(csp_eth_interface_data_t * ifdata, csp_packet_t * buffer, } prev = packet; - packet = next; + packet = packet->next; } }