commit c1693af46aeee640d5a88dfb5e684acb206fff31
author Jeremy Kerr <jk@ozlabs.org> Mon Aug 05 14:30:59 2019 +0800
committer Jeremy Kerr <jk@ozlabs.org> Thu Oct 24 18:32:21 2019 +1100
tree 0bcc6154bc30b0f718ecd83525c6877611b2f914
parent 9a3da81c780182f163ffec87a58ece8442f06330

core: Free packet in mctp_bus_rx

Change mctp_bus_rx to take ownership of a received packet, freeing it
after use.

This prevents potential leaks in binding implementations.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Change-Id: I76c427df7ffa5172e77eccad682325c1376c424f

core.c

diff --git a/core.c b/core.c
index baf60a8..2a9784e 100644
--- a/core.c
+++ b/core.c
@@ -262,7 +262,7 @@
 
 	if (hdr->dest != bus->eid)
 		/* @todo: non-local packet routing */
-		return;
+		goto out;
 
 	flags = hdr->flags_seq_tag & (MCTP_HDR_FLAG_SOM | MCTP_HDR_FLAG_EOM);
 	tag = (hdr->flags_seq_tag >> MCTP_HDR_TAG_SHIFT) & MCTP_HDR_TAG_MASK;
@@ -300,7 +300,7 @@
 	case MCTP_HDR_FLAG_EOM:
 		ctx = mctp_msg_ctx_lookup(mctp, hdr->src, tag);
 		if (!ctx)
-			return;
+			goto out;
 
 		exp_seq = (ctx->last_seq + 1) % 4;
 
@@ -309,7 +309,7 @@
 				"Sequence number %d does not match expected %d",
 				seq, exp_seq);
 			mctp_msg_ctx_drop(ctx);
-			return;
+			goto out;
 		}
 
 		rc = mctp_msg_ctx_add_pkt(ctx, pkt);
@@ -325,7 +325,7 @@
 		/* Neither SOM nor EOM */
 		ctx = mctp_msg_ctx_lookup(mctp, hdr->src, tag);
 		if (!ctx)
-			return;
+			goto out;
 
 		exp_seq = (ctx->last_seq + 1) % 4;
 		if (exp_seq != seq) {
@@ -333,18 +333,20 @@
 				"Sequence number %d does not match expected %d",
 				seq, exp_seq);
 			mctp_msg_ctx_drop(ctx);
-			return;
+			goto out;
 		}
 
 		rc = mctp_msg_ctx_add_pkt(ctx, pkt);
 		if (rc) {
 			mctp_msg_ctx_drop(ctx);
-			return;
+			goto out;
 		}
 		ctx->last_seq = seq;
 
 		break;
 	}
+out:
+	mctp_pktbuf_free(pkt);
 }
 
 static int mctp_packet_tx(struct mctp_bus *bus,

libmctp.h

diff --git a/libmctp.h b/libmctp.h
index f0633e3..ecf78c6 100644
--- a/libmctp.h
+++ b/libmctp.h
@@ -96,6 +96,10 @@
 
 void mctp_binding_set_tx_enabled(struct mctp_binding *binding, bool enable);
 
+/*
+ * Receive a packet from binding to core. Takes ownership of pkt, free()-ing it
+ * after use.
+ */
 void mctp_bus_rx(struct mctp_binding *binding, struct mctp_pktbuf *pkt);
 
 /* environment-specific allocation */

serial.c

diff --git a/serial.c b/serial.c
index 2d955cb..4ad259f 100644
--- a/serial.c
+++ b/serial.c
@@ -143,7 +143,6 @@
 	if (valid)
 		mctp_bus_rx(&serial->binding, pkt);
 
-	mctp_pktbuf_free(pkt);
 	serial->rx_pkt = NULL;
 }
 

tests/test-utils.c

diff --git a/tests/test-utils.c b/tests/test-utils.c
index f8bea2f..71de8a3 100644
--- a/tests/test-utils.c
+++ b/tests/test-utils.c
@@ -39,7 +39,6 @@
 	assert(pkt);
 	memcpy(mctp_pktbuf_hdr(pkt), buf, len);
 	mctp_bus_rx(&test->binding, pkt);
-	mctp_pktbuf_free(pkt);
 }
 
 void mctp_binding_test_register_bus(struct mctp_binding_test *binding,