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 DF131429838 for ; Tue, 5 May 2026 12:44:25 +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=1777985068; cv=none; b=k9HrsuOr2S2BRdfuHJz+FNNmYOf/vml11IBE2F8pTayJilmqQT8qN7oRkZUdLSYH7OWl3ufwKMIM4Xk+9y9oTQQiyYEa/7L59OIS0QhpyPhHGbmvs0Pf1jznrSKvsaLzg1xv/azhr7rizH7IoCSWsuEi+PEs3y9uIAVExvbrHdI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777985068; c=relaxed/simple; bh=hz98aOu66uNVwuTZiRLja0C8i6pImQOrnxxcY7S6SyA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Z0baWACuV8zpnUbL4RuF+s2bjBwG/TPIlRP2sOOE+XAiPgG0D8UMmGhFt29nX31WIm0OLXlGiZ6DID3MxqaWKYxFLEmVOT2zS3YEszfcuHaKHfKKdsGnCpUNGuLNFhtLLTNqRob65S2PbBcklVfuHJpV690Gn4XbgjIc4MwOMLc= 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=gk8gCQ/s; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=byF3Z4NU; 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="gk8gCQ/s"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="byF3Z4NU" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1777985065; 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=jwSGsuiyqii/zr4WOQ6nXZFN/nOITIhLrEP6HdZcP9o=; b=gk8gCQ/sbMz0pBGYqo4aYfuFtiQ0V8NpEIrTTV2nvy1AJQB3nzXA6kMknZqrZXkUoCVxFH Fk33DfMoRTEOMlVQVmOvmNkKUxylyc8e4yhwj1U8LNbB0vdzYwCREIGG/bC7sqpJm9JUN+ gu9aCCvy4VzI7I4E9aiLf7jOCJU+iMA= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-272-sFC4CUhvPamaJeKtOdvzAw-1; Tue, 05 May 2026 08:44:23 -0400 X-MC-Unique: sFC4CUhvPamaJeKtOdvzAw-1 X-Mimecast-MFC-AGG-ID: sFC4CUhvPamaJeKtOdvzAw_1777985063 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-44d83e45febso1676383f8f.0 for ; Tue, 05 May 2026 05:44:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1777985062; x=1778589862; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=jwSGsuiyqii/zr4WOQ6nXZFN/nOITIhLrEP6HdZcP9o=; b=byF3Z4NUEDg841ZEoHUmvCNTzJvR+1RkguBk03pNuHAPFut56h3rF3Z6rhBy2M2/eX lRTxHBSTOsp4B/F3py0jWUuB2T/cKWiYEpLTKy+/TIzX7dVwjQ+Wc3C2JB3ZYWjBVknz SI/14BsPswu07ortmuBvosS9Ez7gWvWwCYNEtICF3vGPjcaWnzYvYEFz/MwPrCP2psuc /npu9MKC28KazhFdInaG6A1hvbGCnGNDQqOI7Y72O8lPmddseVK3aIWgeuJM1QIE6j3j ideBFHaIwZX5xoIOMB8UngkTaBjOutOb8vj1uzAC9skDJp2LWXr6upb4zspNsSSa2ssY nHog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777985062; x=1778589862; h=in-reply-to:content-disposition: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; bh=jwSGsuiyqii/zr4WOQ6nXZFN/nOITIhLrEP6HdZcP9o=; b=iEmlYoZFcSoQ+ZDpgKQyreN+GSdeP4Op3SXdHq4qm8mLGK/QYfJ8yIs0ovCOEHfdIn 7j5FGHzYbptjp+m2PGcez4ruaa29YuRI07WHlHVx1WIn7Rg+8JnYMuXEGDxEMucN2bLO /ArrhKrfXxsTJCE24eIN/IjcRFiJrw2bAmXLETTuJ+B6b99GWRrg4aC3+mr2ZLt3EMvD /V8mjYd2gaunfDfrHIFacZGmZCoi3NVI1RUm/Rf3BZUPAq6XFj6C0nBoSz/ayqXfc8GN Av0ADH/PLDy06OjSf7AEpHaJn0zNmIrAgcFzuIQRoJq1S6tXOf/08hMZazNzEfrD+Af1 bpZg== X-Forwarded-Encrypted: i=1; AFNElJ9CBVIa+tbYfR9xZN5Cgp9wq3z4wdJ1iQS7NDfvVa+UtnQm/IIWQs4XgwWMLKq2gc7o2LPh5RAeYT8zvF0=@vger.kernel.org X-Gm-Message-State: AOJu0YxMm9OG0hmHzUlsKWdEDg3t8C549YkAE1yXlPVA3CxeGHpinWCi QOTFdGinS7W9/wIvt1gHYXGTT/9Adz4dt3cWgc9gwl+JHRk/l1YJcAaSjyrJY4LVCSTssoibs6I EXREzMOf8XdSJm9vNt5bzWaea0tI2CdBIBJjOziOTHBq0v+yTFjsR48ZVAh8LHWJS3A== X-Gm-Gg: AeBDies4EMnk18dlnSBASwYVf6HIMg4hB3FQXe8LwxF4C4Xew1i+4XbPPZDe+ZECItO 8Axql8XHv9sDo9bgWDtYysDBzfyul3Rv4ytNw9CxtxDMTWKGDwIAziTmXvBCM6iM8TGbtJb2nC9 nqW3gRgCmH2GZfOnZjg50ieD2bCeAh2Ar8cKvi1pcFFlng3Nu/IY+M1I4q1GcYNE0M9Rx1kTEKz Up+2smuVazW7NI1se+k9fCp26wBXASRMYAz3GUgh5FEGgTLRPYJAc/njVFw5P0+nN9Appk3qo+U qmp9Lsf99/sFYT+J1dlXfBW9W3o4qrynd6rJzm/5rrKuOP9qSZV0ACn/29vQ4vKiUbXtCaVG8Pz k3RICoqr5h3uUTWrPqGQ0edq17sHGolyEkp0vYEKUWcPNyJ2LPHxi06eCW7engBceDvC1B2d1QA == X-Received: by 2002:a05:6000:186b:b0:449:b7dd:4a75 with SMTP id ffacd0b85a97d-44fdee4405cmr6456581f8f.20.1777985062386; Tue, 05 May 2026 05:44:22 -0700 (PDT) X-Received: by 2002:a05:6000:186b:b0:449:b7dd:4a75 with SMTP id ffacd0b85a97d-44fdee4405cmr6456485f8f.20.1777985061805; Tue, 05 May 2026 05:44:21 -0700 (PDT) Received: from sgarzare-redhat (host-87-11-6-2.retail.telecomitalia.it. [87.11.6.2]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-4505238e6e0sm4243514f8f.6.2026.05.05.05.44.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 May 2026 05:44:21 -0700 (PDT) Date: Tue, 5 May 2026 14:44:16 +0200 From: Stefano Garzarella To: Paolo Abeni , Arseniy Krasnov , Bobby Eshleman Cc: stefanha@redhat.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, mst@redhat.com, jasowang@redhat.com, xuanzhuo@linux.alibaba.com, eperezma@redhat.com, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, horms@kernel.org, Yiqi Sun , kvm@vger.kernel.org, virtualization@lists.linux.dev Subject: Re: [PATCH] vsock/virtio: fix vsockmon info leak in non-linear tap copy Message-ID: References: <20260430071110.380509-1-sunyiqixm@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; format=flowed Content-Disposition: inline In-Reply-To: CCing Arseniy and Bobby. On Tue, May 05, 2026 at 12:26:21PM +0200, Paolo Abeni wrote: >On 4/30/26 9:11 AM, Yiqi Sun wrote: >> vsockmon mirrors packets through virtio_transport_build_skb(), which >> builds a new skb and copies the payload into it. For non-linear skbs, >> this goes through virtio_transport_copy_nonlinear_skb(). >> >> Helper manually initializes a iov_iter, but leaves iov_iter.count unset. >> As a result, skb_copy_datagram_iter() sees zero writable bytes >> in the destination iterator and copies no payload data. >> >> This becomes an info leak because virtio_transport_build_skb() has >> already reserved payload_len bytes in the new skb with skb_put(). The >> skb is then returned to the tap path with that payload area still >> uninitialized, so userspace reading from a vsockmon device can observe >> heap contents and potentially kernel address. >> >> Fix it by initializing iov_iter.count to the number of bytes to copy. >> >> Fixes: 4b0bf10eb077 ("vsock/virtio: non-linear skb handling for tap") >> Signed-off-by: Yiqi Sun >> --- >> net/vmw_vsock/virtio_transport_common.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c >> index 416d533f493d..6b26ee57ccab 100644 >> --- a/net/vmw_vsock/virtio_transport_common.c >> +++ b/net/vmw_vsock/virtio_transport_common.c >> @@ -152,7 +152,7 @@ static void virtio_transport_copy_nonlinear_skb(const struct sk_buff *skb, >> iov_iter.nr_segs = 1; >> >> to_copy = min_t(size_t, len, skb->len); >> - >> + iov_iter.count = to_copy; >> skb_copy_datagram_iter(skb, VIRTIO_VSOCK_SKB_CB(skb)->offset, >> &iov_iter, to_copy); > >@Stefano, @Stefan, the patch LGTM, but sashiko pointed out to a >pre-existing issue you should probably want to address: > >> to_copy = min_t(size_t, len, skb->len); >Does this length calculation account for the offset when a packet is >split across multiple transmissions? >If a packet is requeued, VIRTIO_VSOCK_SKB_CB(skb)->offset is increased, >but to_copy still evaluates to the full length of the skb. Yep, I just checked and vhost-vsock is the only place where we call virtio_transport_deliver_tap_pkt() wiht an offset != 0, but I agree that we should also fix it. Looking better in net/vmw_vsock/virtio_transport_common.c I think this is a regression, indeed we have this comment in virtio_transport_build_skb(): /* A packet could be split to fit the RX buffer, so we can retrieve * the payload length from the header and the buffer pointer taking * care of the offset in the original packet. */ pkt_hdr = virtio_vsock_hdr(pkt); Before commit 71dc9ec9ac7d ("virtio/vsock: replace virtio_vsock_pkt with sk_buff") we read the payload lenght from the header that is always set to the right value before delivering the packet to the tap. From that commit, we don't to consider the offset anymore since we started to use `len` from the skb, so IMO we should go back to what we did before it, I mean: payload_len = le32_to_cpu(pkt->hdr.len); @Bobby do you remember why we did that change? Or if you see any issue going back to what we did initially? Also IMO we should avoid to set all the iov_iter fields by hand and start to use iov_iter_kvec(). Plus, we can just use skb_copy_datagram_iter() in any case, like we already do in vhost-vsock, since it already handles linear vs non linear. At the end I mean something like this: @@ -171,7 +150,7 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque) * care of the offset in the original packet. */ pkt_hdr = virtio_vsock_hdr(pkt); - payload_len = pkt->len; + payload_len = le32_to_cpu(pkt_hdr->len); skb = alloc_skb(sizeof(*hdr) + sizeof(*pkt_hdr) + payload_len, GFP_ATOMIC); @@ -214,13 +193,17 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque) skb_put_data(skb, pkt_hdr, sizeof(*pkt_hdr)); if (payload_len) { - if (skb_is_nonlinear(pkt)) { - void *data = skb_put(skb, payload_len); + struct iov_iter iov_iter; + struct kvec kvec; + void *data = skb_put(skb, payload_len); - virtio_transport_copy_nonlinear_skb(pkt, data, payload_len); - } else { - skb_put_data(skb, pkt->data, payload_len); - } + kvec.iov_base = data; + kvec.iov_len = payload_len; + iov_iter_kvec(&iov_iter, READ, &kvec, 1, payload_len); + + skb_copy_datagram_iter(pkt, + VIRTIO_VSOCK_SKB_CB(pkt)->offset, + &iov_iter, payload_len); } return skb; And removing virtio_transport_copy_nonlinear_skb(). If you agree, I can send a proper series with these changes that should fix the issue reported by Yiqi Sun introduced by commit 4b0bf10eb077 ("vsock/virtio: non-linear skb handling for tap") and the issue introduced by commit 71dc9ec9ac7d ("virtio/vsock: replace virtio_vsock_pkt with sk_buff"). Thanks, Stefano