From: Peter Maydell <peter.mayd...@linaro.org>
The smc91c111 uses packet numbers as an index into its internal
s->data[][] array. Valid packet numbers are between 0 and 3, but
the code does not generally check this, and there are various
places where the guest can hand us an arbitrary packet number
and cause an out-of-bounds access to the data array.
Add validation of packet numbers. The datasheet is not very
helpful about how guest errors like this should be handled:
it says nothing on the subject, and none of the documented
error conditions are relevant. We choose to log the situation
with LOG_GUEST_ERROR and silently ignore the attempted operation.
In the places where we are about to access the data[][] array
using a packet number and we know the number is valid because
we got it from somewhere that has already validated, we add
an assert() to document that belief.
Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
Message-ID: <20250228174802.1945417-2-peter.mayd...@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
hw/net/smc91c111.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c
index 0e13dfa18b2..2295c6acf25 100644
--- a/hw/net/smc91c111.c
+++ b/hw/net/smc91c111.c
@@ -118,6 +118,11 @@ static const VMStateDescription vmstate_smc91c111 = {
#define RS_TOOSHORT 0x0400
#define RS_MULTICAST 0x0001
+static inline bool packetnum_valid(int packet_num)
+{
+ return packet_num >= 0 && packet_num < NUM_PACKETS;
+}
+
/* Update interrupt status. */
static void smc91c111_update(smc91c111_state *s)
{
@@ -218,6 +223,17 @@ static void smc91c111_pop_tx_fifo_done(smc91c111_state *s)
/* Release the memory allocated to a packet. */
static void smc91c111_release_packet(smc91c111_state *s, int packet)
{
+ if (!packetnum_valid(packet)) {
+ /*
+ * Data sheet doesn't document behaviour in this guest error
+ * case, and there is no error status register to report it.
+ * Log and ignore the attempt.
+ */
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "smc91c111: attempt to release invalid packet %d\n",
+ packet);
+ return;
+ }
s->allocated &= ~(1 << packet);
if (s->tx_alloc == 0x80)
smc91c111_tx_alloc(s);
@@ -239,6 +255,8 @@ static void smc91c111_do_tx(smc91c111_state *s)
return;
for (i = 0; i < s->tx_fifo_len; i++) {
packetnum = s->tx_fifo[i];
+ /* queue_tx checked the packet number was valid */
+ assert(packetnum_valid(packetnum));
p = &s->data[packetnum][0];
/* Set status word. */
*(p++) = 0x01;
@@ -287,6 +305,17 @@ static void smc91c111_do_tx(smc91c111_state *s)
/* Add a packet to the TX FIFO. */
static void smc91c111_queue_tx(smc91c111_state *s, int packet)
{
+ if (!packetnum_valid(packet)) {
+ /*
+ * Datasheet doesn't document behaviour in this error case, and
+ * there's no error status register we could report it in.
+ * Log and ignore.
+ */
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "smc91c111: attempt to queue invalid packet %d\n",
+ packet);
+ return;
+ }
if (s->tx_fifo_len == NUM_PACKETS)
return;
s->tx_fifo[s->tx_fifo_len++] = packet;
@@ -457,6 +486,13 @@ static void smc91c111_writeb(void *opaque, hwaddr offset,
n = s->rx_fifo[0];
else
n = s->packet_num;
+ if (!packetnum_valid(n)) {
+ /* Datasheet doesn't document what to do here */
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "smc91c111: attempt to write data to invalid
packet %d\n",
+ n);
+ return;
+ }
p = s->ptr & 0x07ff;
if (s->ptr & 0x4000) {
s->ptr = (s->ptr & 0xf800) | ((s->ptr + 1) & 0x7ff);
@@ -605,6 +641,13 @@ static uint32_t smc91c111_readb(void *opaque, hwaddr
offset)
n = s->rx_fifo[0];
else
n = s->packet_num;
+ if (!packetnum_valid(n)) {
+ /* Datasheet doesn't document what to do here */
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "smc91c111: attempt to read data from
invalid packet %d\n",
+ n);
+ return 0;
+ }
p = s->ptr & 0x07ff;
if (s->ptr & 0x4000) {
s->ptr = (s->ptr & 0xf800) | ((s->ptr + 1) & 0x07ff);
@@ -713,6 +756,8 @@ static ssize_t smc91c111_receive(NetClientState *nc, const
uint8_t *buf, size_t
return -1;
s->rx_fifo[s->rx_fifo_len++] = packetnum;
+ /* allocate_packet() will not hand us back an invalid packet number */
+ assert(packetnum_valid(packetnum));
p = &s->data[packetnum][0];
/* ??? Multicast packets? */
status = 0;
--
2.47.1
