Re: kern/58870: ipmi(4): misaligned buffer in get_sdr_partial

[Taylor R Campbell <riastradh%NetBSD.org@localhost>]

The attached patch tries to address this as proposed.

(I don't have time to test this myself so I don't plan to commit it
at the moment unless someone else wants to test it.)

This could conceivably affect Edgar's get_sdr_partial error at boot
but it seems unlikely (nor do I have any better theories about what
has caused that to happen).

# HG changeset patch
# User Taylor R Campbell <riastradh%NetBSD.org@localhost>
# Date 1733247801 0
#      Tue Dec 03 17:43:21 2024 +0000
# Branch trunk
# Node ID 42c04f18f85a06e870f0a3cf1ad1e2acaf576bc3
# Parent  702bc3da2819f628a6d103b38eaa0e3cbfa40593
# EXP-Topic riastradh-pr56568-ipmidelay
ipmi(4): Avoid misaligned buffer access in get_sdr_partial.

PR kern/58870: ipmi(4): misaligned buffer in get_sdr_partial

diff --git a/sys/dev/ipmi.c b/sys/dev/ipmi.c
--- a/sys/dev/ipmi.c
+++ b/sys/dev/ipmi.c
@@ -1109,29 +1109,44 @@ static int
 get_sdr_partial(struct ipmi_softc *sc, uint16_t recordId, uint16_t reserveId,
     uint8_t offset, uint8_t length, void *buffer, uint16_t *nxtRecordId)
 {
-	uint8_t	cmd[256 + 8];
+	union {
+		struct {
+			uint16_t	reserveId;
+			uint16_t	recordId;
+			uint8_t		offset;
+			uint8_t		length;
+		} __packed	cmd;
+		struct {
+			uint16_t	nxtRecordId;
+			uint8_t		data[262];
+		} __packed	msg;
+	}		u;
 	int		len;
 
-	((uint16_t *) cmd)[0] = reserveId;
-	((uint16_t *) cmd)[1] = recordId;
-	cmd[4] = offset;
-	cmd[5] = length;
+	__CTASSERT(sizeof(u) == 256 + 8);
+	__CTASSERT(sizeof(u.cmd) == 6);
+	__CTASSERT(offsetof(typeof(u.msg), data) == 2);
+
+	u.cmd.reserveId = reserveId;
+	u.cmd.recordId = recordId;
+	u.cmd.offset = offset;
+	u.cmd.length = length;
 	mutex_enter(&sc->sc_cmd_mtx);
-	if (ipmi_sendcmd(sc, BMC_SA, 0, STORAGE_NETFN, STORAGE_GET_SDR, 6,
-	    cmd)) {
+	if (ipmi_sendcmd(sc, BMC_SA, 0, STORAGE_NETFN, STORAGE_GET_SDR,
+		sizeof(u.cmd), &u.cmd)) {
 		mutex_exit(&sc->sc_cmd_mtx);
 		aprint_error_dev(sc->sc_dev, "%s: sendcmd fails\n", __func__);
 		return -1;
 	}
-	if (ipmi_recvcmd(sc, 8 + length, &len, cmd)) {
+	if (ipmi_recvcmd(sc, 8 + length, &len, &u.msg)) {
 		mutex_exit(&sc->sc_cmd_mtx);
 		aprint_error_dev(sc->sc_dev, "%s: recvcmd fails\n", __func__);
 		return -1;
 	}
 	mutex_exit(&sc->sc_cmd_mtx);
 	if (nxtRecordId)
-		*nxtRecordId = *(uint16_t *) cmd;
-	memcpy(buffer, cmd + 2, len - 2);
+		*nxtRecordId = u.msg.nxtRecordId;
+	memcpy(buffer, u.msg.data, len - offsetof(typeof(u.msg), data));
 
 	return 0;
 }