On 27.03.2023 18:22, Juergen Gross wrote:
On 27.03.23 17:38, Jan Beulich wrote:
On 27.03.2023 12:07, Juergen Gross wrote:
On 27.03.23 11:49, Jan Beulich wrote:
On 27.03.2023 10:36, Juergen Gross wrote:
@@ -539,6 +553,13 @@ static int xenvif_tx_check_gop(struct xenvif_queue *queue, pending_idx = copy_pending_idx(skb, i); newerr = (*gopp_copy)->status;
/* Split copies need to be handled together. */if (XENVIF_TX_CB(skb)->split_mask & (1U << i)) {(*gopp_copy)++;if (!newerr)newerr = (*gopp_copy)->status;}It isn't guaranteed that a slot may be split only once, is it? Assuming a
I think it is guaranteed.
No slot can cover more than XEN_PAGE_SIZE bytes due to the grants being restricted to that size. There is no way how such a data packet could cross 2 page boundaries.
In the end the problem isn't the copies for the linear area not crossing multiple page boundaries, but the copies for a single request slot not doing so. And this can't happen IMO.
You're thinking of only well-formed requests. What about said request providing a large size with only tiny fragments? xenvif_get_requests() will happily process such, creating bogus grant-copy ops. But them failing once submitted to Xen will be only after damage may already have occurred (from bogus updates of internal state; the logic altogether is too involved for me to be convinced that nothing bad can happen).
There are sanity checks after each relevant RING_COPY_REQUEST() call, which will bail out if "(txp->offset + txp->size) > XEN_PAGE_SIZE" (the first one is after the call of xenvif_count_requests(), as this call will decrease the size of the request, the other check is in xenvif_count_requests()).
Oh, indeed - that's the check I've been overlooking. (The messages logged there could do with also mentioning "Cross page boundary", like the one in xenvif_count_requests() does.)
Jan