git: 00c07d9559c6 - main - twsi: make data receiving code safer

The branch main has been updated by avg:

URL: https://cgit.FreeBSD.org/src/commit/?id=00c07d9559c6197957b00811a0b29876a5c8b573

commit 00c07d9559c6197957b00811a0b29876a5c8b573
Author:     Andriy Gapon <avg@FreeBSD.org>
AuthorDate: 2021-11-26 08:34:42 +0000
Commit:     Andriy Gapon <avg@FreeBSD.org>
CommitDate: 2021-11-26 14:18:51 +0000

    twsi: make data receiving code safer
    
    Assert that we are not receiving data beyond the requested length.
    Assert that we have not NACK-ed incoming data prematurely.
    Abort the current transfer if the incoming data is NACK-ed or not
    NACK-ed unexpectedly.
    
    Add debug logging of received data to complement logging of sent data.
    
    MFC after:      3 weeks
---
 sys/dev/iicbus/twsi/twsi.c | 62 ++++++++++++++++++++++++++++++----------------
 1 file changed, 41 insertions(+), 21 deletions(-)

diff --git a/sys/dev/iicbus/twsi/twsi.c b/sys/dev/iicbus/twsi/twsi.c
index b94396883be5..2f77e2dc69c3 100644
--- a/sys/dev/iicbus/twsi/twsi.c
+++ b/sys/dev/iicbus/twsi/twsi.c
@@ -671,37 +671,57 @@ twsi_intr(void *arg)
 		break;
 
 	case TWSI_STATUS_DATA_RD_ACK:
-		debugf(sc, "Ack received after receiving data\n");
-		sc->msgs[sc->msg_idx].buf[sc->recv_bytes++] = TWSI_READ(sc, sc->reg_data);
-		debugf(sc, "msg_len=%d recv_bytes=%d\n", sc->msgs[sc->msg_idx].len, sc->recv_bytes);
+		debugf(sc, "Received and ACK-ed data\n");
+		KASSERT(sc->recv_bytes < sc->msgs[sc->msg_idx].len,
+		    ("receiving beyond the end of buffer"));
+
+		sc->msgs[sc->msg_idx].buf[sc->recv_bytes] =
+		    TWSI_READ(sc, sc->reg_data);
+		debugf(sc, "Received byte %d (of %d) = 0x%x\n",
+		    sc->recv_bytes,
+		    sc->msgs[sc->msg_idx].len,
+		    sc->msgs[sc->msg_idx].buf[sc->recv_bytes]);
+		sc->recv_bytes++;
 
 		/* If we only have one byte left, disable ACK */
-		if (sc->msgs[sc->msg_idx].len - sc->recv_bytes == 1)
+		if (sc->msgs[sc->msg_idx].len - sc->recv_bytes == 1) {
 			sc->control_val &= ~TWSI_CONTROL_ACK;
-		if (sc->msgs[sc->msg_idx].len == sc->recv_bytes) {
-			debugf(sc, "Done with msg %d\n", sc->msg_idx);
-			sc->msg_idx++;
-			if (sc->msg_idx == sc->nmsgs - 1) {
-				debugf(sc, "No more msgs\n");
-				transfer_done = 1;
-				sc->error = 0;
-			}
+		} else if (sc->msgs[sc->msg_idx].len == sc->recv_bytes) {
+			/*
+			 * We should not have ACK-ed the last byte.
+			 * The protocol state machine is in invalid state.
+			 */
+			debugf(sc, "RX all but asked for more?\n");
+			twsi_error(sc, IIC_ESTATUS);
 		}
-		TWSI_WRITE(sc, sc->reg_control, sc->control_val);
 		break;
 
 	case TWSI_STATUS_DATA_RD_NOACK:
-		if (sc->msgs[sc->msg_idx].len - sc->recv_bytes == 1) {
-			sc->msgs[sc->msg_idx].buf[sc->recv_bytes++] = TWSI_READ(sc, sc->reg_data);
-			debugf(sc, "Done RX data, send stop (2)\n");
-			if (!(sc->msgs[sc->msg_idx].flags & IIC_M_NOSTOP))
+		debugf(sc, "Received and NACK-ed data\n");
+		KASSERT(sc->recv_bytes == sc->msgs[sc->msg_idx].len - 1,
+		    ("sent NACK before receiving all requested data"));
+		sc->msgs[sc->msg_idx].buf[sc->recv_bytes] =
+		    TWSI_READ(sc, sc->reg_data);
+		debugf(sc, "Received byte %d (of %d) = 0x%x\n",
+		    sc->recv_bytes,
+		    sc->msgs[sc->msg_idx].len,
+		    sc->msgs[sc->msg_idx].buf[sc->recv_bytes]);
+		sc->recv_bytes++;
+
+		if (sc->msgs[sc->msg_idx].len == sc->recv_bytes) {
+			debugf(sc, "Done RX data\n");
+			if (!(sc->msgs[sc->msg_idx].flags & IIC_M_NOSTOP)) {
+				debugf(sc, "Send STOP\n");
 				TWSI_WRITE(sc, sc->reg_control,
 				    sc->control_val | TWSI_CONTROL_STOP);
+			}
 		} else {
-			debugf(sc, "No ack when receiving data, sending stop anyway\n");
-			if (!(sc->msgs[sc->msg_idx].flags & IIC_M_NOSTOP))
-				TWSI_WRITE(sc, sc->reg_control,
-				    sc->control_val | TWSI_CONTROL_STOP);
+			/*
+			 * We should not have NACK-ed yet.
+			 * The protocol state machine is in invalid state.
+			 */
+			debugf(sc, "NACK-ed before receving all bytes?\n");
+			twsi_error(sc, IIC_ESTATUS);
 		}
 		sc->transfer = 0;
 		transfer_done = 1;