From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37336) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8nY5-0002nZ-VO for qemu-devel@nongnu.org; Tue, 04 Sep 2012 03:27:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T8nXv-0001WK-1Z for qemu-devel@nongnu.org; Tue, 04 Sep 2012 03:27:49 -0400 Received: from mout.web.de ([212.227.17.11]:58659) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8nXu-0001WB-MN for qemu-devel@nongnu.org; Tue, 04 Sep 2012 03:27:38 -0400 Message-ID: <5045AD67.7040300@web.de> Date: Tue, 04 Sep 2012 09:27:35 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1346704484-15110-1-git-send-email-sw@weilnetz.de> In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigE5A5175B9154E630635A9941" Subject: Re: [Qemu-devel] [PATCH] slirp: Fix error reported by static code analysis and remove wrong type casts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Stefan Weil , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigE5A5175B9154E630635A9941 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 2012-09-03 23:08, Peter Maydell wrote: > On 3 September 2012 21:34, Stefan Weil wrote: >> Report from smatch: >> slirp/tcp_subr.c:127 tcp_respond(17) error: >> we previously assumed 'tp' could be null (see line 124) >> >> Fix this by checking 'tp' before reading its elements. >> >> The type casts of pointers to long are not related to the smatch repor= t >> but happened to be near that code. Those type casts are not allowed >> when sizeof(pointer) !=3D sizeof(long). >=20 > I think these would be better in a separate patch. >> @@ -124,7 +124,7 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti,= struct mbuf *m, >> if (tp) >> win =3D sbspace(&tp->t_socket->so_rcv); >> if (m =3D=3D NULL) { >> - if ((m =3D m_get(tp->t_socket->slirp)) =3D=3D NULL) >> + if (tp && (m =3D m_get(tp->t_socket->slirp)) =3D=3D NU= LL) >> return; >> tlen =3D 0; >> m->m_data +=3D IF_MAXLINKHDR; >=20 > This doesn't look quite right -- now if tp is NULL we will skip > the assignment to m and dereference a NULL pointer a few lines > further on, right? This would be correct: if (!tp || ..) return >=20 > I suspect that we need either to be passed our Slirp* explicitly rather= > than via tp, or we have to enforce that tcp_respond() is called with > a non-NULL struct tcpcb*. I think the only cases where tp can be non-NU= LL > at the moment are the two calls from the dropwithreset code in tcp_inpu= t(). Indeed, this is a "XXX Should never fail" case - according to the code that checks tp at the call site. But as no one seriously understands slirp details, we are better safe than sorry. Jan --------------enigE5A5175B9154E630635A9941 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iEYEARECAAYFAlBFrWgACgkQitSsb3rl5xR0MACfQrnrPEnTh/iHc5jhGERvbKyA MVgAoObE/uwwVNu33zSmHSwdx2PC9KH7 =XZyh -----END PGP SIGNATURE----- --------------enigE5A5175B9154E630635A9941--