Fix test-i2c leak, and mctp_pktbuf storage alignment
- Ensure mctp_pktbuf storage is correctly aligned
- Deallocate mctp and i2c instances to avoid failure with asan.
These previously succeeded in CI so are both fixed in this commit.
Fixes: e5b941d9d764 ("i2c: Add binding for MCTP over I2C transport")
Fixes: 4a09e1dc4883 ("core: Reuse buffers for tx, allow message pools")
Change-Id: I747bfff6faf3a5b0a982ae266bcef02ecbc4ee8a
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
diff --git a/core.c b/core.c
index 7b84d5a..c5ded9d 100644
--- a/core.c
+++ b/core.c
@@ -8,6 +8,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <stdalign.h>
#undef pr_fmt
#define pr_fmt(fmt) "core: " fmt
@@ -65,6 +66,8 @@
struct mctp_pktbuf *mctp_pktbuf_init(struct mctp_binding *binding,
void *storage)
{
+ assert((size_t)storage % alignof(struct mctp_pktbuf) == 0);
+
size_t size =
binding->pkt_size + binding->pkt_header + binding->pkt_trailer;
struct mctp_pktbuf *buf = (struct mctp_pktbuf *)storage;
diff --git a/i2c-internal.h b/i2c-internal.h
index 95ab81d..eed788e 100644
--- a/i2c-internal.h
+++ b/i2c-internal.h
@@ -35,10 +35,8 @@
uint8_t own_addr;
- uint8_t tx_storage[sizeof(struct mctp_i2c_hdr) +
- MCTP_PKTBUF_SIZE(I2C_BTU)] __attribute((aligned(8)));
- uint8_t rx_storage[sizeof(struct mctp_i2c_hdr) +
- MCTP_PKTBUF_SIZE(I2C_BTU)] __attribute((aligned(8)));
+ uint8_t tx_storage[MCTP_PKTBUF_SIZE(I2C_BTU)] PKTBUF_STORAGE_ALIGN;
+ uint8_t rx_storage[MCTP_PKTBUF_SIZE(I2C_BTU)] PKTBUF_STORAGE_ALIGN;
mctp_i2c_tx_fn tx_fn;
void *tx_ctx;
diff --git a/libmctp.h b/libmctp.h
index 2fdd812..96ee975 100644
--- a/libmctp.h
+++ b/libmctp.h
@@ -11,6 +11,7 @@
#include <stdbool.h>
#include <stdint.h>
#include <stddef.h>
+#include <stdalign.h>
typedef uint8_t mctp_eid_t;
@@ -58,6 +59,7 @@
#define MCTP_PKTBUF_SIZE(payload) \
(MCTP_PACKET_SIZE(payload) + sizeof(struct mctp_pktbuf))
+#define PKTBUF_STORAGE_ALIGN __attribute((aligned(alignof(struct mctp_pktbuf))))
struct mctp;
struct mctp_bus;
@@ -65,7 +67,9 @@
/* Initialise a mctp_pktbuf in static storage. Should not be freed.
* Storage must be sized to fit the binding,
- * MCTP_PKTBUF_SIZE(binding->pkt_size + binding->pkt_header + binding->pkt_trailer) */
+ * MCTP_PKTBUF_SIZE(binding->pkt_size + binding->pkt_header + binding->pkt_trailer).
+ * storage must be aligned to alignof(struct mctp_pktbuf),
+ * use PKTBUF_STORAGE_ALIGN macro */
struct mctp_pktbuf *mctp_pktbuf_init(struct mctp_binding *binding,
void *storage);
/* Allocate and initialise a mctp_pktbuf. Should be freed with
@@ -175,7 +179,7 @@
/**
* @tx: Binding function to transmit one packet on the interface
* @tx_storage: A buffer for transmitting packets. Must be sized
- * as MCTP_PKTBUF_SIZE(mtu).
+ * as MCTP_PKTBUF_SIZE(mtu) and 8 byte aligned.
* Return:
* * 0 - Success, pktbuf can be released
* * -EMSGSIZE - Packet exceeds binding MTU, pktbuf must be dropped
diff --git a/serial.c b/serial.c
index d79c99c..b61991a 100644
--- a/serial.c
+++ b/serial.c
@@ -44,7 +44,7 @@
/* receive buffer and state */
uint8_t rxbuf[1024];
struct mctp_pktbuf *rx_pkt;
- uint8_t rx_storage[MCTP_PKTBUF_SIZE(SERIAL_BTU)];
+ uint8_t rx_storage[MCTP_PKTBUF_SIZE(SERIAL_BTU)] PKTBUF_STORAGE_ALIGN;
uint8_t rx_exp_len;
uint16_t rx_fcs;
uint16_t rx_fcs_calc;
@@ -62,7 +62,7 @@
/* temporary transmit buffer */
uint8_t txbuf[256];
/* used by the MCTP stack */
- uint8_t tx_storage[MCTP_PKTBUF_SIZE(SERIAL_BTU)];
+ uint8_t tx_storage[MCTP_PKTBUF_SIZE(SERIAL_BTU)] PKTBUF_STORAGE_ALIGN;
};
#define binding_to_serial(b) \
diff --git a/tests/test-utils.c b/tests/test-utils.c
index 5e78474..2a346fa 100644
--- a/tests/test-utils.c
+++ b/tests/test-utils.c
@@ -16,7 +16,7 @@
* the local EID as the destination */
struct mctp_binding_test {
struct mctp_binding binding;
- uint8_t tx_storage[MCTP_PKTBUF_SIZE(MCTP_BTU)];
+ uint8_t tx_storage[MCTP_PKTBUF_SIZE(MCTP_BTU)] PKTBUF_STORAGE_ALIGN;
};
static int mctp_binding_test_tx(struct mctp_binding *b, struct mctp_pktbuf *pkt)
diff --git a/tests/test_bridge.c b/tests/test_bridge.c
index 73705ba..40d5fb6 100644
--- a/tests/test_bridge.c
+++ b/tests/test_bridge.c
@@ -19,7 +19,7 @@
int rx_count;
int tx_count;
uint8_t last_pkt_data;
- uint8_t tx_storage[MCTP_PKTBUF_SIZE(MCTP_BTU)];
+ uint8_t tx_storage[MCTP_PKTBUF_SIZE(MCTP_BTU)] PKTBUF_STORAGE_ALIGN;
};
struct test_ctx {
diff --git a/tests/test_cmds.c b/tests/test_cmds.c
index 2646b8c..b4bf913 100644
--- a/tests/test_cmds.c
+++ b/tests/test_cmds.c
@@ -60,7 +60,7 @@
assert(test_endpoint != NULL);
assert(callback_ctx != NULL);
- uint8_t tx_storage[MCTP_PKTBUF_SIZE(MCTP_BTU)];
+ uint8_t tx_storage[MCTP_PKTBUF_SIZE(MCTP_BTU)] PKTBUF_STORAGE_ALIGN;
memset(test_binding, 0, sizeof(*test_binding));
test_binding->name = "test";
test_binding->version = 1;
diff --git a/tests/test_i2c.c b/tests/test_i2c.c
index 067d551..89e7744 100644
--- a/tests/test_i2c.c
+++ b/tests/test_i2c.c
@@ -259,5 +259,10 @@
test_neigh_expiry(tx_test, rx_test);
+ free(tx_test->i2c);
+ free(rx_test->i2c);
+ mctp_destroy(tx_test->mctp);
+ mctp_destroy(rx_test->mctp);
+
return 0;
}