[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] net/e1000e_core: make sure RDH never exceeds RDT in e1000e_r

From: Jason Wang
Subject: Re: [PATCH] net/e1000e_core: make sure RDH never exceeds RDT in e1000e_ring_advance()
Date: Wed, 11 Nov 2020 16:54:24 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0

On 2020/11/10 下午5:06, Mauro Matteo Cascella wrote:
On Mon, Nov 9, 2020 at 3:38 AM Jason Wang <jasowang@redhat.com> wrote:

On 2020/11/5 下午6:56, Mauro Matteo Cascella wrote:
The e1000e_write_packet_to_guest() function iterates over a set of
receive descriptors by advancing rx descriptor head register (RDH) from
its initial value to rx descriptor tail register (RDT). The check in
e1000e_ring_empty() is responsible for detecting whether RDH has reached
RDT, terminating the loop if that's the case. Additional checks have
been added in the past to deal with bogus values submitted by the guest
to prevent possible infinite loop. This is done by "wrapping around" RDH
at some point and detecting whether it assumes the original value during
the loop.

However, when e1000e is configured to use the packet split feature, RDH is
incremented by two instead of one, as the packet split descriptors are
32 bytes while regular descriptors are 16 bytes. A malicious or buggy
guest may set RDT to an odd value and transmit only null RX descriptors.
This corner case would prevent RDH from ever matching RDT, leading to an
infinite loop. This patch adds a check in e1000e_ring_advance() to make
sure RDH does never exceed RDT.

This issue was independently reported by Gaoning Pan (Zhejiang University)
and Cheolwoo Myung.

Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
Reported-by: Gaoning Pan <gaoning.pgn@antgroup.com>
Reported-by: Cheolwoo Myung <330cjfdn@gmail.com>

   hw/net/e1000e_core.c | 4 ++++
   1 file changed, 4 insertions(+)

diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index bcd186cac5..4c4d14b6ed 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -831,6 +831,10 @@ e1000e_ring_advance(E1000ECore *core, const 
E1000E_RingInfo *r, uint32_t count)
       core->mac[r->dh] += count;

+    if (core->mac[r->dh] > core->mac[r->dt]) {
+        core->mac[r->dh] = core->mac[r->dt];
+    }
       if (core->mac[r->dh] * E1000_RING_DESC_LEN >= core->mac[r->dlen]) {
           core->mac[r->dh] = 0;
Hi Jason,

A question here.

When count > 1, is this correct to reset dh here?


My understanding is that wrapping at (or above) RDLEN is the correct
behavior regardless of count. I don't see why count > 1 should modify
this behavior. I'm not sure, though. Anyway, this patch fixes the
above reproducer, so I'm adding a Tested-by line here.

Tested-by: Mauro Matteo Cascella <mcascell@redhat.com>

Thank you,
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0




reply via email to

[Prev in Thread] Current Thread [Next in Thread]