Browse Source
In e1000e_write_packet_to_guest() we attempt to ensure that we don't write more of a packet to a descriptor than will fit in the guest configured receive buffers. However, this code does not allow for the "packet split" feature. When packet splitting is enabled, the first of up to 4 buffers in the descriptor is used for the packet header only, with the payload going into buffers 2, 3 and 4. Our length check only checks against the total sizes of all 4 buffers, which meant that if an incoming packet was large enough to fit in (1 + 2 + 3 + 4) but not into (2 + 3 + 4) and packet splitting was enabled, we would run into the assertion in e1000e_write_hdr_frag_to_rx_buffers() that we had enough buffers for the data: qemu-system-i386: ../../hw/net/e1000e_core.c:1418: void e1000e_write_payload_frag_to_rx_buffers(E1000ECore *, hwaddr *, E1000EBAState *, const char *, dma_addr_t): Assertion `bastate->cur_idx < MAX_PS_BUFFERS' failed. A malicious guest could provoke this assertion by configuring the device into loopback mode, and then sending itself a suitably sized packet into a suitably arrange rx descriptor. The code also fails to deal with the possibility that the descriptor buffers are sized such that the trailing checksum word does not fit into the last descriptor which has actual data, which might also trigger this assertion. Rework the length handling to use two variables: * desc_size is the total amount of data DMA'd to the guest for the descriptor being processed in this iteration of the loop * rx_desc_buf_size is the total amount of space left in it As we copy data to the guest (packet header, payload, checksum), update these two variables. (Previously we attempted to calculate desc_size once at the top of the loop, but this is too difficult to do correctly.) Then we can use the variables to ensure that we clamp the amount of copied payload data to the remaining space in the descriptor's buffers, even if we've used one of the buffers up in the packet-split code, and we can tell whether we have enough space for the full checksum word in this descriptor or whether we're going to need to split that to the following descriptor. I have included comments that hopefully help to make the loop logic a little clearer. Cc: qemu-stable@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/issues/537 Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Jason Wang <jasowang@redhat.com>pull/307/head
committed by
Jason Wang
1 changed files with 47 additions and 14 deletions
Loading…
Reference in new issue