Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 30 May 2017 16:15:52 +0000 (UTC)
From:      Alan Somers <asomers@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org
Subject:   svn commit: r319222 - stable/10/sys/dev/xen/netback
Message-ID:  <201705301615.v4UGFqZR010214@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: asomers
Date: Tue May 30 16:15:52 2017
New Revision: 319222
URL: https://svnweb.freebsd.org/changeset/base/319222

Log:
  MFC r314148, r314150
  
  r314148:
  Misc Coverity fixes in xnb(4)
  
  Most of these are null pointer dereferences or missing error checks in the
  unit tests. One is a missing error check in xnb_attach_failed. None can
  cause real problems in running systems.
  
  Reported by:	Coverity
  CIDs:		1092469 1092468 1092467 2092466 1092465 1092512 1092511 1092510
  CIDs:		1092510 1092509 1092508 1092507
  Reviewed by:	royger
  Sponsored by:	Spectra Logic Corp
  Differential Revision:	https://reviews.freebsd.org/D9234
  
  r314150:
  Fix the xnb(4) unit tests
  
  One test was inadvertently expecting a bug in the kernel's sscanf
  implementation circa 2012. I don't know when that bug got fixed.
  
  Reported by:	royger
  Reviewed by:	royger
  Sponsored by:	Spectra Logic Corp
  Differential Revision:	https://reviews.freebsd.org/D9766

Modified:
  stable/10/sys/dev/xen/netback/netback.c
  stable/10/sys/dev/xen/netback/netback_unit_tests.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/dev/xen/netback/netback.c
==============================================================================
--- stable/10/sys/dev/xen/netback/netback.c	Tue May 30 16:09:54 2017	(r319221)
+++ stable/10/sys/dev/xen/netback/netback.c	Tue May 30 16:15:52 2017	(r319222)
@@ -1111,14 +1111,13 @@ xnb_attach_failed(struct xnb_softc *xnb, int err, cons
 	xs_vprintf(XST_NIL, xenbus_get_node(xnb->dev),
 		  "hotplug-error", fmt, ap_hotplug);
 	va_end(ap_hotplug);
-	xs_printf(XST_NIL, xenbus_get_node(xnb->dev),
+	(void)xs_printf(XST_NIL, xenbus_get_node(xnb->dev),
 		  "hotplug-status", "error");
 
 	xenbus_dev_vfatal(xnb->dev, err, fmt, ap);
 	va_end(ap);
 
-	xs_printf(XST_NIL, xenbus_get_node(xnb->dev),
-		  "online", "0");
+	(void)xs_printf(XST_NIL, xenbus_get_node(xnb->dev), "online", "0");
 	xnb_detach(xnb->dev);
 }
 

Modified: stable/10/sys/dev/xen/netback/netback_unit_tests.c
==============================================================================
--- stable/10/sys/dev/xen/netback/netback_unit_tests.c	Tue May 30 16:09:54 2017	(r319221)
+++ stable/10/sys/dev/xen/netback/netback_unit_tests.c	Tue May 30 16:15:52 2017	(r319222)
@@ -1227,6 +1227,10 @@ xnb_txpkt2gnttab_2cluster(char *buffer, size_t buflen)
 	xnb_ring2pkt(&pkt, &xnb_unit_pvt.txb, xnb_unit_pvt.txb.req_cons);
 
 	pMbuf = xnb_pkt2mbufc(&pkt, xnb_unit_pvt.ifp);
+	XNB_ASSERT(pMbuf != NULL);
+	if (pMbuf == NULL)
+		return;
+
 	n_entries = xnb_txpkt2gnttab(&pkt, pMbuf, xnb_unit_pvt.gnttab,
 	    &xnb_unit_pvt.txb, DOMID_FIRST_RESERVED);
 
@@ -1271,8 +1275,7 @@ xnb_txpkt2gnttab_2cluster(char *buffer, size_t buflen)
 		/* should never get here */
 		XNB_ASSERT(0);
 	}
-	if (pMbuf != NULL)
-		m_freem(pMbuf);
+	m_freem(pMbuf);
 }
 
 
@@ -1494,15 +1497,14 @@ xnb_mbufc2pkt_2short(char *buffer, size_t buflen) {
 	struct mbuf *mbufc, *mbufc2;
 
 	mbufc = m_getm(NULL, size1, M_WAITOK, MT_DATA);
-	mbufc->m_flags |= M_PKTHDR;
-	if (mbufc == NULL) {
-		XNB_ASSERT(mbufc != NULL);
+	XNB_ASSERT(mbufc != NULL);
+	if (mbufc == NULL)
 		return;
-	}
+	mbufc->m_flags |= M_PKTHDR;
 
 	mbufc2 = m_getm(mbufc, size2, M_WAITOK, MT_DATA);
+	XNB_ASSERT(mbufc2 != NULL);
 	if (mbufc2 == NULL) {
-		XNB_ASSERT(mbufc2 != NULL);
 		safe_m_freem(&mbufc);
 		return;
 	}
@@ -1537,11 +1539,10 @@ xnb_mbufc2pkt_long(char *buffer, size_t buflen) {
 	struct mbuf *mbufc, *m;
 
 	mbufc = m_getm(NULL, size, M_WAITOK, MT_DATA);
-	mbufc->m_flags |= M_PKTHDR;
-	if (mbufc == NULL) {
-		XNB_ASSERT(mbufc != NULL);
+	XNB_ASSERT(mbufc != NULL);
+	if (mbufc == NULL)
 		return;
-	}
+	mbufc->m_flags |= M_PKTHDR;
 
 	mbufc->m_pkthdr.len = size;
 	size_remaining = size;
@@ -1576,10 +1577,9 @@ xnb_mbufc2pkt_extra(char *buffer, size_t buflen) {
 	struct mbuf *mbufc, *m;
 
 	mbufc = m_getm(NULL, size, M_WAITOK, MT_DATA);
-	if (mbufc == NULL) {
-		XNB_ASSERT(mbufc != NULL);
+	XNB_ASSERT(mbufc != NULL);
+	if (mbufc == NULL)
 		return;
-	}
 
 	mbufc->m_flags |= M_PKTHDR;
 	mbufc->m_pkthdr.len = size;
@@ -1619,11 +1619,10 @@ xnb_mbufc2pkt_nospace(char *buffer, size_t buflen) {
 	int error;
 
 	mbufc = m_getm(NULL, size, M_WAITOK, MT_DATA);
-	mbufc->m_flags |= M_PKTHDR;
-	if (mbufc == NULL) {
-		XNB_ASSERT(mbufc != NULL);
+	XNB_ASSERT(mbufc != NULL);
+	if (mbufc == NULL)
 		return;
-	}
+	mbufc->m_flags |= M_PKTHDR;
 
 	mbufc->m_pkthdr.len = size;
 	size_remaining = size;
@@ -1840,10 +1839,9 @@ xnb_rxpkt2rsp_extra(char *buffer, size_t buflen)
 	struct netif_extra_info *ext;
 
 	mbufc = m_getm(NULL, size, M_WAITOK, MT_DATA);
-	if (mbufc == NULL) {
-		XNB_ASSERT(mbufc != NULL);
+	XNB_ASSERT(mbufc != NULL);
+	if (mbufc == NULL)
 		return;
-	}
 
 	mbufc->m_flags |= M_PKTHDR;
 	mbufc->m_pkthdr.len = size;
@@ -1974,11 +1972,10 @@ xnb_rxpkt2rsp_2short(char *buffer, size_t buflen) {
 	struct mbuf *mbufc;
 
 	mbufc = m_getm(NULL, size1, M_WAITOK, MT_DATA);
-	mbufc->m_flags |= M_PKTHDR;
-	if (mbufc == NULL) {
-		XNB_ASSERT(mbufc != NULL);
+	XNB_ASSERT(mbufc != NULL);
+	if (mbufc == NULL)
 		return;
-	}
+	mbufc->m_flags |= M_PKTHDR;
 
 	m_getm(mbufc, size2, M_WAITOK, MT_DATA);
 	XNB_ASSERT(mbufc->m_next != NULL);
@@ -2451,7 +2448,7 @@ xnb_sscanf_hhu(char *buffer, size_t buflen)
 	for (i = 0; i < 12; i++)
 		dest[i] = 'X';
 
-	sscanf(mystr, "%hhu", &dest[4]);
+	XNB_ASSERT(sscanf(mystr, "%hhu", &dest[4]) == 1);
 	for (i = 0; i < 12; i++)
 		XNB_ASSERT(dest[i] == (i == 4 ? 137 : 'X'));
 }
@@ -2469,7 +2466,7 @@ xnb_sscanf_hhd(char *buffer, size_t buflen)
 	for (i = 0; i < 12; i++)
 		dest[i] = 'X';
 
-	sscanf(mystr, "%hhd", &dest[4]);
+	XNB_ASSERT(sscanf(mystr, "%hhd", &dest[4]) == 1);
 	for (i = 0; i < 12; i++)
 		XNB_ASSERT(dest[i] == (i == 4 ? -27 : 'X'));
 }
@@ -2487,7 +2484,7 @@ xnb_sscanf_lld(char *buffer, size_t buflen)
 	for (i = 0; i < 3; i++)
 		dest[i] = (long long)0xdeadbeefdeadbeef;
 
-	sscanf(mystr, "%lld", &dest[1]);
+	XNB_ASSERT(sscanf(mystr, "%lld", &dest[1]) == 1);
 	for (i = 0; i < 3; i++)
 		XNB_ASSERT(dest[i] == (i != 1 ? (long long)0xdeadbeefdeadbeef :
 		    -123456789012345));
@@ -2506,7 +2503,7 @@ xnb_sscanf_llu(char *buffer, size_t buflen)
 	for (i = 0; i < 3; i++)
 		dest[i] = (long long)0xdeadbeefdeadbeef;
 
-	sscanf(mystr, "%llu", &dest[1]);
+	XNB_ASSERT(sscanf(mystr, "%llu", &dest[1]) == 1);
 	for (i = 0; i < 3; i++)
 		XNB_ASSERT(dest[i] == (i != 1 ? (long long)0xdeadbeefdeadbeef :
 		    12802747070103273189ull));
@@ -2528,10 +2525,10 @@ xnb_sscanf_hhn(char *buffer, size_t buflen)
 	for (i = 0; i < 12; i++)
 		dest[i] = (unsigned char)'X';
 
-	sscanf(mystr,
+	XNB_ASSERT(sscanf(mystr,
 	    "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f"
 	    "202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f"
-	    "404142434445464748494a4b4c4d4e4f%hhn", &dest[4]);
+	    "404142434445464748494a4b4c4d4e4f%hhn", &dest[4]) == 0);
 	for (i = 0; i < 12; i++)
 		XNB_ASSERT(dest[i] == (i == 4 ? 160 : 'X'));
 }

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201705301615.v4UGFqZR010214>