Re: [PATCH v2] net/e1000e_core: adjust count if RDH exceeds RDT in e1000e_ring_advance()

[Jason Wang <jasowang@redhat.com>]

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


>
> Thank you,