114 lines
4.6 KiB
Diff
114 lines
4.6 KiB
Diff
From f66f7d35b84a06629ef4d5d13b0f9e06076191d4 Mon Sep 17 00:00:00 2001
|
|
From: Michael Hanselmann <public@hansmi.ch>
|
|
Date: Sat, 29 May 2021 16:54:38 +0200
|
|
Subject: [PATCH] Release memory after handling packet
|
|
|
|
These memory leaks were found with the usbredirparser fuzzer.
|
|
|
|
The API transfers ownership of the data allocation only for the *_packet
|
|
callbacks (also documented in "usbredirparser.h"). The hello packet handler
|
|
already freed the payload while the others didn't.
|
|
|
|
In addition destroying the parser structure must also destroy the read data
|
|
buffer.
|
|
|
|
With these changes it's finally possible to run in excess of 1.7 million fuzzing
|
|
iterations without noticable memory leaks. LeakSanitizer no longer reports after
|
|
a few iterations.
|
|
|
|
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
|
|
---
|
|
usbredirparser/usbredirparser.c | 21 +++++++++++++++++----
|
|
1 file changed, 17 insertions(+), 4 deletions(-)
|
|
|
|
diff --git a/usbredirparser/usbredirparser.c b/usbredirparser/usbredirparser.c
|
|
index 67303d0..5d19e38 100644
|
|
--- a/usbredirparser/usbredirparser.c
|
|
+++ b/usbredirparser/usbredirparser.c
|
|
@@ -22,6 +22,7 @@
|
|
|
|
#include <assert.h>
|
|
#include <stddef.h>
|
|
+#include <stdbool.h>
|
|
#include <stdio.h>
|
|
#include <stdlib.h>
|
|
#include <stdarg.h>
|
|
@@ -281,7 +282,6 @@ static void usbredirparser_handle_hello(struct usbredirparser *parser_pub,
|
|
}
|
|
usbredirparser_verify_caps(parser, parser->peer_caps, "peer");
|
|
parser->have_peer_caps = 1;
|
|
- free(data);
|
|
|
|
INFO("Peer version: %s, using %d-bits ids", buf,
|
|
usbredirparser_using_32bits_ids(parser_pub) ? 32 : 64);
|
|
@@ -773,7 +773,8 @@ static int usbredirparser_verify_type_header(
|
|
return 1; /* Verify ok */
|
|
}
|
|
|
|
-static void usbredirparser_call_type_func(struct usbredirparser *parser_pub)
|
|
+static void usbredirparser_call_type_func(struct usbredirparser *parser_pub,
|
|
+ bool *data_ownership_transferred)
|
|
{
|
|
struct usbredirparser_priv *parser =
|
|
(struct usbredirparser_priv *)parser_pub;
|
|
@@ -912,26 +913,31 @@ static void usbredirparser_call_type_func(struct usbredirparser *parser_pub)
|
|
parser->type_header);
|
|
break;
|
|
case usb_redir_control_packet:
|
|
+ *data_ownership_transferred = true;
|
|
parser->callb.control_packet_func(parser->callb.priv, id,
|
|
(struct usb_redir_control_packet_header *)parser->type_header,
|
|
parser->data, parser->data_len);
|
|
break;
|
|
case usb_redir_bulk_packet:
|
|
+ *data_ownership_transferred = true;
|
|
parser->callb.bulk_packet_func(parser->callb.priv, id,
|
|
(struct usb_redir_bulk_packet_header *)parser->type_header,
|
|
parser->data, parser->data_len);
|
|
break;
|
|
case usb_redir_iso_packet:
|
|
+ *data_ownership_transferred = true;
|
|
parser->callb.iso_packet_func(parser->callb.priv, id,
|
|
(struct usb_redir_iso_packet_header *)parser->type_header,
|
|
parser->data, parser->data_len);
|
|
break;
|
|
case usb_redir_interrupt_packet:
|
|
+ *data_ownership_transferred = true;
|
|
parser->callb.interrupt_packet_func(parser->callb.priv, id,
|
|
(struct usb_redir_interrupt_packet_header *)parser->type_header,
|
|
parser->data, parser->data_len);
|
|
break;
|
|
case usb_redir_buffered_bulk_packet:
|
|
+ *data_ownership_transferred = true;
|
|
parser->callb.buffered_bulk_packet_func(parser->callb.priv, id,
|
|
(struct usb_redir_buffered_bulk_packet_header *)parser->type_header,
|
|
parser->data, parser->data_len);
|
|
@@ -944,6 +950,7 @@ int usbredirparser_do_read(struct usbredirparser *parser_pub)
|
|
struct usbredirparser_priv *parser =
|
|
(struct usbredirparser_priv *)parser_pub;
|
|
int r, header_len, type_header_len, data_len;
|
|
+ bool data_ownership_transferred;
|
|
uint8_t *dest;
|
|
|
|
header_len = usbredirparser_get_header_len(parser_pub);
|
|
@@ -1028,8 +1035,14 @@ int usbredirparser_do_read(struct usbredirparser *parser_pub)
|
|
r = usbredirparser_verify_type_header(parser_pub,
|
|
parser->header.type, parser->type_header,
|
|
parser->data, parser->data_len, 0);
|
|
- if (r)
|
|
- usbredirparser_call_type_func(parser_pub);
|
|
+ data_ownership_transferred = false;
|
|
+ if (r) {
|
|
+ usbredirparser_call_type_func(parser_pub,
|
|
+ &data_ownership_transferred);
|
|
+ }
|
|
+ if (!data_ownership_transferred) {
|
|
+ free(parser->data);
|
|
+ }
|
|
parser->header_read = 0;
|
|
parser->type_header_len = 0;
|
|
parser->type_header_read = 0;
|
|
--
|
|
1.8.3.1
|
|
|