hw/net/smc91c111: Sanitize packet numbers

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-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20250228174802.1945417-2-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
This commit is contained in:
Peter Maydell 2025-02-28 17:47:59 +00:00 committed by Philippe Mathieu-Daudé
parent 822405b1fe
commit 2fa3a5b946

View file

@ -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;