Received-SPF: pass (google.com: domain of linux-kernel+bounces-153397-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Mon, 22 Apr 2024 09:00:31 -0400
From: Stefan Hajnoczi <stefanha@redhat.com>
To: Jeongjun Park <aha310510@gmail.com>
Cc: mst@redhat.com, jasowang@redhat.com, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, sgarzare@redhat.com,
	syzbot+6c21aeb59d0e82eb2782@syzkaller.appspotmail.com,
	syzkaller-bugs@googlegroups.com, virtualization@lists.linux.dev,
	Arseny Krasnov <arseny.krasnov@kaspersky.com>
Subject: Re: [PATCH virt] virt: fix uninit-value in vhost_vsock_dev_open
Message-ID: <20240422130031.GA77895@fedora>
References: <20240420060450-mutt-send-email-mst@kernel.org>
 <20240421030606.80385-1-aha310510@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="fbjDzR1gbQWxLtRv"
Content-Disposition: inline
In-Reply-To: <20240421030606.80385-1-aha310510@gmail.com>


--fbjDzR1gbQWxLtRv
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sun, Apr 21, 2024 at 12:06:06PM +0900, Jeongjun Park wrote:
> static bool vhost_transport_seqpacket_allow(u32 remote_cid)
> {
> ....
> 	vsock =3D vhost_vsock_get(remote_cid);
>=20
> 	if (vsock)
> 		seqpacket_allow =3D vsock->seqpacket_allow;
> ....
> }
>=20
> I think this is due to reading a previously created uninitialized=20
> vsock->seqpacket_allow inside vhost_transport_seqpacket_allow(),=20
> which is executed by the function pointer present in the if statement.

CCing Arseny, author of commit ced7b713711f ("vhost/vsock: support
SEQPACKET for transport").

Looks like a genuine bug in the commit. vhost_vsock_set_features() sets
seqpacket_allow to true when the feature is negotiated. The assumption
is that the field defaults to false.

The rest of the vhost_vsock.ko code is written to initialize the
vhost_vsock fields, so you could argue seqpacket_allow should just be
explicitly initialized to false.

However, eliminating this class of errors by zeroing seems reasonable in
this code path. vhost_vsock_dev_open() is not performance-critical.

Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

--fbjDzR1gbQWxLtRv
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEhpWov9P5fNqsNXdanKSrs4Grc8gFAmYmX24ACgkQnKSrs4Gr
c8g0xgf9GAsOG1fK6cXKiusz/7vFucJOezY6SjiRsy4zMiPpUTRRsDAiaA6akyQ6
5Bnmnr1HjGGwXOeROMJfGWohjhlMCsKKCSiWT8rnPOOIQsw5yc6tgd2aqmtmbD3q
APFtGlvHmQwlRE/6/aAvkoUoTMQuS3pL3pKQqKNDgboo+lJAdKp97SiOypeLxiZ1
dYYtujTVn46aivjD/Sdm731fGFRadB/1cBPLlDioac/22TmBckx2cpOhxC3Ggoo3
tYXRtVSMb0ICOBe2Krf2sE+TqAr96/JF8DFWlag5040Bp1VfoO54OfbnvPDtu3bG
gQF5MIPjdLy7jk1A50VuhR7p7ILV6A==
=pV2Q
-----END PGP SIGNATURE-----

--fbjDzR1gbQWxLtRv--