From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A7F182D46B3 for ; Sat, 11 Jul 2026 15:20:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783783239; cv=none; b=K3wbYr+AmuUZnY8BgFg1sqDgV0xhdFnzKoKDFmklMZexO8uT5DVfWAH4YP+B6Rj8/uBJQ9Mb21FYAZ1xhiOYRK5giPLpGmPuOgYJEwo7cUlabiKjtIafpbzAXgGhENXrNfzMloWbY3SsgcL8XCcfnwiKjGPWFjrVddUfJW8uZO0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783783239; c=relaxed/simple; bh=8PCyBlIWQjZCCPrTVN7xE+pF7wIpZzqRFJWJCP5kxbc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=B9peYw6jbqqDx92uGrjimt8Nz5/vwBmOkdcybGc7xs32Bg/jn6mlCHbJ6nJW1UKOA7lJ8+JiDHhpT0jrBKCnaR8tStNH/p50gY6m/EhND3LVORhnG7mY9fADHVtALsR14jhWZ8rTfSb9w1AIP/Ed1ISRWJimUzRlKBcuxPJtCRo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=agEM9sdV; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=Ie9uGZO3; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="agEM9sdV"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="Ie9uGZO3" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1783783236; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=QmUM+RWb25XiR9LGFNREMs6O7PLt7AQAUsEOfZqyWvs=; b=agEM9sdVXpRra684f77JCukQwUxJ/mh8evYBUPIhdT3NpRggmxKOEVzAqZ4TE9baCn0VJi xoQ0BciPTXzdikNZAA0PdSa0HlrBtu6g0Lfl427NVxV+XmQ+52LY3nlYEcVAAdH38+XGev YWl/9SjYpEijgrFQ3G6Gtcunuo9PT/4= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-630-_Wj339tHNf24c-YgAj4wUQ-1; Sat, 11 Jul 2026 11:20:34 -0400 X-MC-Unique: _Wj339tHNf24c-YgAj4wUQ-1 X-Mimecast-MFC-AGG-ID: _Wj339tHNf24c-YgAj4wUQ_1783783233 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-493c20d0468so30795865e9.1 for ; Sat, 11 Jul 2026 08:20:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1783783233; x=1784388033; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=QmUM+RWb25XiR9LGFNREMs6O7PLt7AQAUsEOfZqyWvs=; b=Ie9uGZO3w5c+yDqEyxr6+FjgGheSV9GOHXMjwQ5WDwRVfGiKDg7juIdEo8b6FsE485 kYaVZNsegQ0Hyw2/OfdQ2iz/izQXH+UjThFH6K+7hcXopBgg1Izdx9JJ2lZqY2V0S8SJ hpwk0CwjP4fPQO1SVPAyXdhS0NOjEbjLCZXpawppN90Pq3LJoyOweQsrQ44KZOCp07YL 29EZxD5PNwOPgkWyCQQfoIzAVF82TY3TQ03oj/SeCX2lb3g6pvom4folg5gcED15Wn7M KQkUVcKy9r/P+G/lyQZV40wOEsbTUr+ua6RiDbSvtRXry3EECIsRvAJ9Wqx4RaFZxkUf f1Dg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783783233; x=1784388033; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=QmUM+RWb25XiR9LGFNREMs6O7PLt7AQAUsEOfZqyWvs=; b=ccgMXFbMhGPZG18PKEcclZ4Itiakh7KkWOQwfMbvRUkQqlLtCaQxLmnUzBansC6XY4 UqhZXBdc0iQG+KKil/zrVgn5wK9oLRGj8CjfVYZG9heYLD9n0GRdIMvT5VX1Fl0D88DD DYuIVIKkgTR3L2Rvmk+iTwiHNesRRQVQcFFsryDN+8q9eHMAD0rXgK69UCQOm4oNdC11 h0Tsa1tW5QYl7oXWNCrVf2eYpjn5MlqLqAfelwN7wPx4L9663Ga3DVNIKFacUzcmc4II 5jXrlYZXoO6hWHEvVgr3+Td0q+d6VrpiTlqN97KwMQCmIvWOAKSogqq/M7EKDOXGZzno XUFA== X-Forwarded-Encrypted: i=1; AHgh+Rr0N/b4on1D6FQLHeDsBJ5reobYe/oItvvmHg9Wl4SlSSb7gZ0897C6aKaTMH2Vm3Qex7HFLWuyUwP2SEk=@vger.kernel.org X-Gm-Message-State: AOJu0YyWslVA2DMdZxFyIPDCFdlWEolpTI29bfd4tb4ebehO7x+SgJYP uYTswFKDjU1OklHsiiHgYpeVNDAZKUwCF1w/7H6k4GniSUpwzBJ4WvbJLAu9y09xNMSajmd8I9Q +KunhIFCMjL3FMP7IYQ5izsU9Ip/CibMN3izbeQmxaWQJ5FGWhbreksQnwMJMeiZ0LA== X-Gm-Gg: AfdE7cncqlaZSQaCuj0dOay0XTJpWfGffO0zeftrKjP0RJoB6Fvu74EpSfOURZ480CG SSMrSph3CzXRjY08ra+VaQHafJWKIG5g38wK8XInLpTjegqiAxOLKTXFaTzqdij2atRLnmEW5n/ lyB3nm9NfQoQbf8efG9dMykFKGjMvkok+NfYoYSWR9dIkFDyiSFh8KGHTvoHpqWv1ADeDpi158i PiGJqwGsowMh6o29ozeg3gXSjHuGLkqr7Wsqf2YS8iGhVqnnQ0wNEz04wevNuSbWcQr0AlagsCZ 3ueiaJp/IcULQQxYs65ORQE66qq2snbkMfbNLCLechleY79+MdExeDDTa8fOIcDlajxemmljU48 bdXc= X-Received: by 2002:a05:600c:3e19:b0:493:f28e:462a with SMTP id 5b1f17b1804b1-493f87e6ba2mr28363945e9.12.1783783233294; Sat, 11 Jul 2026 08:20:33 -0700 (PDT) X-Received: by 2002:a05:600c:3e19:b0:493:f28e:462a with SMTP id 5b1f17b1804b1-493f87e6ba2mr28363605e9.12.1783783232734; Sat, 11 Jul 2026 08:20:32 -0700 (PDT) Received: from redhat.com ([185.81.125.101]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-493eb6ff6casm266286555e9.5.2026.07.11.08.20.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 11 Jul 2026 08:20:31 -0700 (PDT) Date: Sat, 11 Jul 2026 11:20:28 -0400 From: "Michael S. Tsirkin" To: Michael Bommarito Cc: Jason Wang , Xuan Zhuo , Eugenio =?iso-8859-1?Q?P=E9rez?= , Andrew Lunn , Jakub Kicinski , Paolo Abeni , virtualization@lists.linux.dev, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH] virtio_net: validate device stats reply records before use Message-ID: <20260711111503-mutt-send-email-mst@kernel.org> References: <20260711150754.2918392-1-michael.bommarito@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260711150754.2918392-1-michael.bommarito@gmail.com> On Sat, Jul 11, 2026 at 11:07:54AM -0400, Michael Bommarito wrote: > __virtnet_get_hw_stats() walks the device statistics reply buffer with > "for (p = reply; p - reply < res_size; p += le16_to_cpu(hdr->size))", > using each record's device-supplied hdr->size as the stride without > checking that a full struct virtio_net_stats_reply_hdr remains, that > hdr->size is nonzero and matches the expected size for hdr->type, or that > the record fits within res_size. A backend that returns hdr->size == 0 > spins the loop forever; a short or oversized size drives out-of-bounds > reads in virtnet_fill_stats(). > > Impact: a malicious or compromised virtio-net backend hangs the CPU > running the guest's device-statistics query in an infinite loop > (hdr->size == 0), or drives an out-of-bounds read of the reply buffer. > This matters most for a confidential guest, where the host is outside the > trust boundary. Why does it "matter most", or at all, there? Host can always deny guest service. In fact, this is how cloud vendors charge their clients, by denying service to whoever did not pay them. > Validate each record before use: require a full header in the remaining > bytes, a nonzero hdr->size that is at least the header size and matches the > size expected for hdr->type, and that the record fits within res_size; stop > the walk otherwise. Add virtnet_stats_reply_size() for the per-type size. I'm all for making things easier to debug even when the device is buggy. But I'm not inclined to add tons of hard to maintain code to that end, and I would be worried broken hosts will come to rely on drivers working around them. > > Fixes: 941168f8b40e ("virtio_net: support device stats") > Cc: stable@vger.kernel.org > Assisted-by: Claude:claude-opus-4-8 > Signed-off-by: Michael Bommarito > --- > drivers/net/virtio_net.c | 42 ++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 40 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 3e2a5876c6c8c..9cbe40d218cc4 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -3532,6 +3532,7 @@ static int virtnet_tx_resize(struct virtnet_info *vi, struct send_queue *sq, > return err; > } > > + > /* > * Send command via the control virtqueue and check status. Commands > * supported by the hypervisor, as indicated by feature bits, should > @@ -3546,6 +3547,7 @@ static bool virtnet_send_command_reply(struct virtnet_info *vi, u8 class, u8 cmd > bool ok; > int ret; > > + > /* Caller should know better */ > BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ)); > we don't need this. > @@ -4927,6 +4929,32 @@ static void virtnet_fill_stats(struct virtnet_info *vi, u32 qid, > } > } > > +static int virtnet_stats_reply_size(u8 type) > +{ > + switch (type) { > + case VIRTIO_NET_STATS_TYPE_REPLY_CVQ: > + return sizeof(struct virtio_net_stats_cvq); > + case VIRTIO_NET_STATS_TYPE_REPLY_RX_BASIC: > + return sizeof(struct virtio_net_stats_rx_basic); > + case VIRTIO_NET_STATS_TYPE_REPLY_RX_CSUM: > + return sizeof(struct virtio_net_stats_rx_csum); > + case VIRTIO_NET_STATS_TYPE_REPLY_RX_GSO: > + return sizeof(struct virtio_net_stats_rx_gso); > + case VIRTIO_NET_STATS_TYPE_REPLY_RX_SPEED: > + return sizeof(struct virtio_net_stats_rx_speed); > + case VIRTIO_NET_STATS_TYPE_REPLY_TX_BASIC: > + return sizeof(struct virtio_net_stats_tx_basic); > + case VIRTIO_NET_STATS_TYPE_REPLY_TX_CSUM: > + return sizeof(struct virtio_net_stats_tx_csum); > + case VIRTIO_NET_STATS_TYPE_REPLY_TX_GSO: > + return sizeof(struct virtio_net_stats_tx_gso); > + case VIRTIO_NET_STATS_TYPE_REPLY_TX_SPEED: > + return sizeof(struct virtio_net_stats_tx_speed); > + default: > + return sizeof(struct virtio_net_stats_reply_hdr); > + } > +} > + > static int __virtnet_get_hw_stats(struct virtnet_info *vi, > struct virtnet_stats_ctx *ctx, > struct virtio_net_ctrl_queue_stats *req, > @@ -4936,7 +4964,7 @@ static int __virtnet_get_hw_stats(struct virtnet_info *vi, > struct scatterlist sgs_in, sgs_out; > void *p; > u32 qid; > - int ok; > + int hdr_size, ok, remaining; > > sg_init_one(&sgs_out, req, req_size); > sg_init_one(&sgs_in, reply, res_size); > @@ -4948,8 +4976,17 @@ static int __virtnet_get_hw_stats(struct virtnet_info *vi, > if (!ok) > return ok; > > - for (p = reply; p - reply < res_size; p += le16_to_cpu(hdr->size)) { > + for (p = reply; p - reply < res_size; p += hdr_size) { > + remaining = res_size - (p - reply); > + if (remaining < sizeof(*hdr)) > + return -EINVAL; > + > hdr = p; > + hdr_size = le16_to_cpu(hdr->size); > + if (hdr_size < virtnet_stats_reply_size(hdr->type) || > + hdr_size > remaining) > + return -EINVAL; > + > qid = le16_to_cpu(hdr->vq_index); > virtnet_fill_stats(vi, qid, ctx, p, false, hdr->type); > } That's a lot of fragile code for unclear benefit. > @@ -7305,3 +7342,4 @@ module_exit(virtio_net_driver_exit); > MODULE_DEVICE_TABLE(virtio, id_table); > MODULE_DESCRIPTION("Virtio network driver"); > MODULE_LICENSE("GPL"); > + > -- > 2.53.0