On Wed, 18 Dec 2024 10:20:47 +0100 Olivier Matz <olivier.m...@6wind.com> wrote:
> Hi, > > On Wed, Dec 18, 2024 at 09:59:05AM +0100, Maxime Coquelin wrote: > > Hi, > > > > On 12/18/24 08:34, Wangyunjian(wangyunjian,TongTu) wrote: > > > > -----Original Message----- > > > > From: Maxime Coquelin [mailto:maxime.coque...@redhat.com] > > > > Sent: Tuesday, December 17, 2024 11:33 PM > > > > To: dev@dpdk.org > > > > Cc: Olivier Matz <olivier.m...@6wind.com>; Maxime Gouin > > > > <maxime.go...@6wind.com>; Maxime Coquelin > > > > <maxime.coque...@redhat.com> > > > > Subject: [PATCH] net/virtio: fix Rx checksum calculation > > > > > > > > From: Olivier Matz <olivier.m...@6wind.com> > > > > > > > > If hdr->csum_start is larger than packet length, the len argument passed > > > > to rte_raw_cksum_mbuf() overflows and causes a segmentation fault. > > > > > > > > Ignore checksum computation in this case. > > > > > > > > CVE-2024-11614 > > > > > > > > Fixes: ca7036b4af3a ("vhost: fix offload flags in Rx path") > > > > > > > > Signed-off-by: Maxime Gouin <maxime.go...@6wind.com> > > > > Signed-off-by: Olivier Matz <olivier.m...@6wind.com> > > > > Reviewed-by: Maxime Coquelin <maxime.coque...@redhat.com> > > > > --- > > > > lib/vhost/virtio_net.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c > > > > index d764d4bc6a..69901ab3b5 100644 > > > > --- a/lib/vhost/virtio_net.c > > > > +++ b/lib/vhost/virtio_net.c > > > > @@ -2823,6 +2823,9 @@ vhost_dequeue_offload(struct virtio_net *dev, > > > > struct virtio_net_hdr *hdr, > > > > */ > > > > uint16_t csum = 0, off; > > > > > > > > + if (hdr->csum_start >= rte_pktmbuf_pkt_len(m)) > > > > + return; > > > > + > > > > > > The hdr->csum_start does two successive reads from user space to read > > > a variable length data structure. The result overflow if the data > > > structure > > > changes between the two reads. > > > > > > We can prevent double fetch issue by using the temporary variable > > > csum_start. > > This is an interesting remark, thanks! > > However, in practical, I'd say that the hdr->csum_start is fetched in a > register > only once if using optimized compilation, because the compiler has no reason > to > think that hdr->csum_start can be modified. > > Olivier True, but security never depend on optimization. Needs a fetch and compiler barrier to be truly safe against compilers.
