An assertion in e1000e_write_payload_frag_to_rx_buffers() attempts to
guard against the calling code accidentally trying to write too much
data to a single RX descriptor, such that the E1000EBAState::cur_idx
indexes off the end of the EB1000BAState::written[] array.
Unfortunately it is overzealous: it asserts that cur_idx is in
range after it has been incremented. This will fire incorrectly
for the case where the guest configures four buffers and exactly
enough bytes are written to fill all four of them.
The only places where we use cur_idx and index in to the written[]
array are the functions e1000e_write_hdr_frag_to_rx_buffers() and
e1000e_write_payload_frag_to_rx_buffers(), so we can rewrite this to
assert before doing the array dereference, rather than asserting
after updating cur_idx.
Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This is just moving the assert to the top, but the patch is
a little more than that to satisfy our "declarations at top
of block" rule.
I haven't seen this assert trip because of this problem (the issue
537 test case does not rely on it), so this is just a "seen on code
inspection" change.
---
hw/net/e1000e_core.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index 471c3ed20b8..46e156a5ddc 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -1392,10 +1392,13 @@ e1000e_write_payload_frag_to_rx_buffers(E1000ECore *core,
dma_addr_t data_len)
{
while (data_len > 0) {
- uint32_t cur_buf_len = core->rxbuf_sizes[bastate->cur_idx];
- uint32_t cur_buf_bytes_left = cur_buf_len -
- bastate->written[bastate->cur_idx];
- uint32_t bytes_to_write = MIN(data_len, cur_buf_bytes_left);
+ uint32_t cur_buf_len, cur_buf_bytes_left, bytes_to_write;
+
+ assert(bastate->cur_idx < MAX_PS_BUFFERS);
+
+ cur_buf_len = core->rxbuf_sizes[bastate->cur_idx];
+ cur_buf_bytes_left = cur_buf_len - bastate->written[bastate->cur_idx];
+ bytes_to_write = MIN(data_len, cur_buf_bytes_left);
trace_e1000e_rx_desc_buff_write(bastate->cur_idx,
ba[bastate->cur_idx],
@@ -1414,8 +1417,6 @@ e1000e_write_payload_frag_to_rx_buffers(E1000ECore *core,
if (bastate->written[bastate->cur_idx] == cur_buf_len) {
bastate->cur_idx++;
}
-
- assert(bastate->cur_idx < MAX_PS_BUFFERS);
}
}
--
2.43.0