qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: qemu-devel@nongnu.org
Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>,
	Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>,
	Jason Wang <jasowang@redhat.com>
Subject: [PATCH 3/3] hw/net/e1000e_core: Adjust e1000e_write_payload_frag_to_rx_buffers() assert
Date: Mon,  3 Nov 2025 17:58:51 +0000	[thread overview]
Message-ID: <20251103175851.428816-4-peter.maydell@linaro.org> (raw)
In-Reply-To: <20251103175851.428816-1-peter.maydell@linaro.org>

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



  parent reply	other threads:[~2025-11-03 18:00 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-03 17:58 [PATCH 0/3] hw/net/e1000e: Fix assertion failures in e1000e_write_payload_frag_to_rx_buffers() Peter Maydell
2025-11-03 17:58 ` [PATCH 1/3] hw/net/e1000e_core: Don't advance desc_offset for NULL buffer RX descriptors Peter Maydell
2025-11-04  6:11   ` Akihiko Odaki
2025-11-04  9:55     ` Peter Maydell
2025-11-04 16:44   ` Philippe Mathieu-Daudé
2025-11-03 17:58 ` [PATCH 2/3] hw/net/e1000e_core: Correct rx oversize packet checks Peter Maydell
2025-11-07  3:50   ` Jason Wang
2025-11-07 10:06     ` Peter Maydell
2025-11-03 17:58 ` Peter Maydell [this message]
2025-11-05  6:21 ` [PATCH 0/3] hw/net/e1000e: Fix assertion failures in e1000e_write_payload_frag_to_rx_buffers() Akihiko Odaki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251103175851.428816-4-peter.maydell@linaro.org \
    --to=peter.maydell@linaro.org \
    --cc=dmitry.fleytman@gmail.com \
    --cc=jasowang@redhat.com \
    --cc=odaki@rsg.ci.i.u-tokyo.ac.jp \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).