On Thu, Nov 19, 2020 at 6:57 AM Jason Wang <jasowang@redhat.com> wrote:
On 2020/11/18 下午4:53, Mauro Matteo Cascella wrote:
On Wed, Nov 18, 2020 at 4:56 AM Jason Wang <jasowang@redhat.com> wrote:
On 2020/11/13 下午6:31, 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 not exceed RDT in a single incremental step, adjusting the count
value accordingly.
Can this patch solve this issue in another way?
https://patchew.org/QEMU/20201111130636.2208620-1-ppandit@redhat.com/
Thanks
Yes, it does work nicely. Still, I think this patch is useful to avoid
possible inconsistent state in e1000e_ring_advance() when count > 1.
So if RDT is odd, it looks to me the following codes in
e1000e_write_packet_to_guest() needs to be fixed as well.
base = e1000e_ring_head_descr(core, rxi);
pci_dma_read(d, base, &desc, core->rx_desc_len);
Otherwise e1000e may try to read out of descriptor ring.
Sorry, I'm not sure I understand what you mean. Isn't the base address
computed from RDH? How can e1000e read out of the descriptor ring if
RDT is odd?
Thanks
On Thu, Nov 19, 2020 at 6:57 AM Jason Wang <jasowang@redhat.com> wrote:
On 2020/11/18 下午4:53, Mauro Matteo Cascella wrote:
On Wed, Nov 18, 2020 at 4:56 AM Jason Wang <jasowang@redhat.com> wrote:
On 2020/11/13 下午6:31, 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 not exceed RDT in a single incremental step, adjusting the count
value accordingly.
Can this patch solve this issue in another way?
https://patchew.org/QEMU/20201111130636.2208620-1-ppandit@redhat.com/
Thanks
Yes, it does work nicely. Still, I think this patch is useful to avoid
possible inconsistent state in e1000e_ring_advance() when count > 1.
So if RDT is odd, it looks to me the following codes in
e1000e_write_packet_to_guest() needs to be fixed as well.
base = e1000e_ring_head_descr(core, rxi);
pci_dma_read(d, base, &desc, core->rx_desc_len);
Otherwise e1000e may try to read out of descriptor ring.
Thanks