lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <4992e7c4-e424-1fce-b508-ff9eaf2fbf2a@suse.com> Date: Tue, 28 Mar 2023 09:50:16 +0200 From: Jan Beulich <jbeulich@...e.com> To: Juergen Gross <jgross@...e.com> Cc: Wei Liu <wei.liu@...nel.org>, Paul Durrant <paul@....org>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, xen-devel@...ts.xenproject.org, stable@...r.kernel.org, linux-kernel@...r.kernel.org, netdev@...r.kernel.org Subject: Re: [PATCH 1/2] xen/netback: don't do grant copy across page boundary 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
Powered by blists - more mailing lists