npcx: CEC: Fix issues with pushing to incoming buffer

When pushing to the circular buffer, the read-offset mutex is no
longer taken, so don't unlock the mutex.

Don't allow writing to the last byte of the buffer. In that case,
the read and write pointers will become equal and the buffer will
be treated as empty.

Add handling for pushing messages of invalid size.

Signed-off-by: Stefan Adolfsson <sadolfsson@chromium.org>

BUG=b:76467407
BRANCH=none
TEST=Turn on/off TV:
ectool cec write 0x40 0x36
ectool cec write 0x04 0x40
Verify that incoming messages still works when turning off TV:
ectool cec read -- -1

Reviewed-on: https://chromium-review.googlesource.com/1068945
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Change-Id: Id207c442fac573430aac0c744ec07fa203074228
Reviewed-on: https://chromium-review.googlesource.com/1070067
Reviewed-by: Stefan Adolfsson <sadolfsson@chromium.org>
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
Tested-by: Stefan Adolfsson <sadolfsson@chromium.org>
Commit-Queue: Stefan Adolfsson <sadolfsson@chromium.org>

1 files changed, 20 insertions, 7 deletions

diff --git a/chip/npcx/cec.c b/chip/npcx/cec.c
index 540fa3c482..bb8f1fd29a 100644
--- a/chip/npcx/cec.c
+++ b/chip/npcx/cec.c
@@ -424,6 +424,9 @@ static int rx_circbuf_push(struct cec_rx_cb *cb, uint8_t *msg, uint8_t msg_len)
 	int i;
 	uint32_t offset;
 
+	if (msg_len > MAX_CEC_MSG_LEN || msg_len == 0)
+		return EC_ERROR_INVAL;
+
 	offset = cb->write_offset;
 	/* Fill in message length last, if successful. Set to zero for now */
 	cb->buf[offset] = 0;
@@ -432,19 +435,27 @@ static int rx_circbuf_push(struct cec_rx_cb *cb, uint8_t *msg, uint8_t msg_len)
 	for (i = 0 ; i < msg_len; i++) {
 		if (offset == cb->read_offset) {
 			/* Buffer full */
-			return -1;
+			return EC_ERROR_OVERFLOW;
 		}
 
 		cb->buf[offset] = msg[i];
 		offset = (offset + 1) % CEC_CIRCBUF_SIZE;
 	}
 
+	/*
+	 * Don't commit if we caught up with read-offset
+	 * since that would indicate an empty buffer
+	 */
+	if (offset == cb->read_offset) {
+		/* Buffer full */
+		return EC_ERROR_OVERFLOW;
+	}
+
 	/* Commit the push */
 	cb->buf[cb->write_offset] = msg_len;
 	cb->write_offset = offset;
-	mutex_unlock(&circbuf_readoffset_mutex);
 
-	return 0;
+	return EC_SUCCESS;
 }
 
 static int rx_circbuf_pop(struct cec_rx_cb *cb, uint8_t *msg, uint8_t *msg_len)
@@ -1158,13 +1169,15 @@ void cec_task(void)
 		if (events & TASK_EVENT_RECEIVED_DATA) {
 			rv = rx_circbuf_push(&cec_rx_cb, cec_rx.msgt.buf,
 					     cec_rx.msgt.byte);
-			if (rv < 0) {
+			if (rv == EC_ERROR_OVERFLOW) {
 				/* Buffer full, prefer the most recent msg */
 				rx_circbuf_flush(&cec_rx_cb);
-				rx_circbuf_push(&cec_rx_cb, cec_rx.msgt.buf,
-						cec_rx.msgt.byte);
+				rv = rx_circbuf_push(&cec_rx_cb,
+						     cec_rx.msgt.buf,
+						     cec_rx.msgt.byte);
 			}
-			mkbp_send_event(EC_MKBP_EVENT_CEC_MESSAGE);
+			if (rv == EC_SUCCESS)
+				mkbp_send_event(EC_MKBP_EVENT_CEC_MESSAGE);
 		}
 	}
 }