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: [email protected] Signed-off-by: Peter Maydell <[email protected]> --- 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
