[PATCH] openbsc[master]: sndcp: Fixups for sndcp layer based on coverity-scan suggest...

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

dexter gerrit-no-reply at lists.osmocom.org
Wed Sep 28 13:21:41 UTC 2016


Review at  https://gerrit.osmocom.org/965

sndcp: Fixups for sndcp layer based on coverity-scan suggestions

- missing break in gprs_sndcp_pcomp.c, line 143
- string overflow in slhc_test.c, line 211
- sizeof mismatch in gprs_sndcp_xid.c, line 1369 and 1378
- mismatching signedness in gprs_sndcp_xid.c, line 1377
- needless < 0 comparison in gprs_sndcp_xid.c, line 477
- needless < 0 comparison in gprs_sndcp_xid.c, line 209
- missing returncode check in v42bis_test.c, line 320
- wrong pointer dereferentialization in gprs_sndcp_comp.c, line 73

Change-Id: I4f9adf251f5119e67ffe76baad6f1f996ac8dbad
---
M openbsc/src/gprs/gprs_sndcp.c
M openbsc/src/gprs/gprs_sndcp_comp.c
M openbsc/src/gprs/gprs_sndcp_pcomp.c
M openbsc/src/gprs/gprs_sndcp_xid.c
M openbsc/tests/slhc/slhc_test.c
M openbsc/tests/v42bis/v42bis_test.c
6 files changed, 13 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/65/965/1

diff --git a/openbsc/src/gprs/gprs_sndcp.c b/openbsc/src/gprs/gprs_sndcp.c
index 1cbeede..59a9cd4 100644
--- a/openbsc/src/gprs/gprs_sndcp.c
+++ b/openbsc/src/gprs/gprs_sndcp.c
@@ -41,7 +41,7 @@
 #include <openbsc/gprs_sndcp_dcomp.h>
 #include <openbsc/gprs_sndcp_comp.h>
 
-#define DEBUG_IP_PACKETS 0	/* 0=Disabled, 1=Enabled */
+#define DEBUG_IP_PACKETS 1	/* 0=Disabled, 1=Enabled */
 
 #if DEBUG_IP_PACKETS == 1
 /* Calculate TCP/IP checksum */
diff --git a/openbsc/src/gprs/gprs_sndcp_comp.c b/openbsc/src/gprs/gprs_sndcp_comp.c
index b13cb8b..cae0039 100644
--- a/openbsc/src/gprs/gprs_sndcp_comp.c
+++ b/openbsc/src/gprs/gprs_sndcp_comp.c
@@ -70,7 +70,7 @@
 		       comp_field->v42bis_params->nsapi,
 		       sizeof(comp_entity->nsapi));
 	} else if (comp_field->v44_params) {
-		comp_entity->nsapi_len = comp_field->v42bis_params->nsapi_len;
+		comp_entity->nsapi_len = comp_field->v44_params->nsapi_len;
 		memcpy(comp_entity->nsapi,
 		       comp_field->v42bis_params->nsapi,
 		       sizeof(comp_entity->nsapi));
diff --git a/openbsc/src/gprs/gprs_sndcp_pcomp.c b/openbsc/src/gprs/gprs_sndcp_pcomp.c
index 5f6fb2c..493b263 100644
--- a/openbsc/src/gprs/gprs_sndcp_pcomp.c
+++ b/openbsc/src/gprs/gprs_sndcp_pcomp.c
@@ -141,6 +141,7 @@
 	switch (pcomp_index) {
 	case 0:
 		type = SL_TYPE_IP;
+		break;
 	case 1:
 		type = SL_TYPE_UNCOMPRESSED_TCP;
 		break;
diff --git a/openbsc/src/gprs/gprs_sndcp_xid.c b/openbsc/src/gprs/gprs_sndcp_xid.c
index 270bdee..a4f9442 100644
--- a/openbsc/src/gprs/gprs_sndcp_xid.c
+++ b/openbsc/src/gprs/gprs_sndcp_xid.c
@@ -206,7 +206,6 @@
 
 	/* Bail if number of ROHC profiles exceeds limit
 	 * (ROHC supports only a maximum of 16 different profiles) */
-	OSMO_ASSERT(params->profile_len >= 0);
 	OSMO_ASSERT(params->profile_len <= 16);
 
 	/* Zero out buffer */
@@ -475,8 +474,7 @@
 		for (i = 0; i < comp_field->comp_len; i++) {
 			/* Check if submitted PCOMP/DCOMP
 			   values are within bounds */
-			if ((comp_field->comp[i] < 0)
-			    || (comp_field->comp[i] > 0x0F))
+			if (comp_field->comp[i] > 0x0F)
 				return -EINVAL;
 
 			if (i & 1) {
@@ -1360,25 +1358,27 @@
 {
 	struct gprs_sndcp_comp_field *comp_field;
 	int i = 0;
+	int rc;
 
 	if (!comp_fields)
 		return -EINVAL;
 	if (!lt)
 		return -EINVAL;
 
-	memset(lt, 0, lt_len * sizeof(lt));
+	memset(lt, 0, sizeof(*lt));
 
 	llist_for_each_entry(comp_field, comp_fields, list) {
 
 		lt[i].entity = comp_field->entity;
 		lt[i].algo = comp_field->algo;
-		lt[i].compclass = gprs_sndcp_get_compression_class(comp_field);
+		rc = gprs_sndcp_get_compression_class(comp_field);
 
-		if (lt[i].compclass < 0) {
-			memset(lt, 0, lt_len * sizeof(lt));
+		if (rc < 0) {
+			memset(lt, 0, sizeof(*lt));
 			return -EINVAL;
 		}
 
+		lt[i].compclass = rc;
 		i++;
 	}
 
diff --git a/openbsc/tests/slhc/slhc_test.c b/openbsc/tests/slhc/slhc_test.c
index e8ea02f..d2e1cd9 100644
--- a/openbsc/tests/slhc/slhc_test.c
+++ b/openbsc/tests/slhc/slhc_test.c
@@ -182,6 +182,8 @@
 		memset(packet, 0, sizeof(packet));
 		memset(packet_compr, 0, sizeof(packet_compr));
 		memset(packet_decompr, 0, sizeof(packet_decompr));
+
+		OSMO_ASSERT(strlen(packets[i]) < sizeof(packet_ascii));
 		strcpy(packet_ascii, packets[i]);
 
 		packet_len =
diff --git a/openbsc/tests/v42bis/v42bis_test.c b/openbsc/tests/v42bis/v42bis_test.c
index 4e05514..7e90785 100644
--- a/openbsc/tests/v42bis/v42bis_test.c
+++ b/openbsc/tests/v42bis/v42bis_test.c
@@ -318,6 +318,7 @@
 	len = strlen(uncompr_packets[packet_id]);
 	testvec = talloc_zero_size(ctx, len);
 	len = osmo_hexparse(uncompr_packets[packet_id], testvec, len);
+	OSMO_ASSERT(len > 0);
 	v42bis(ctx, V42BIS_COMPRESSION_MODE_DYNAMIC, testvec, len);
 	v42bis(ctx, V42BIS_COMPRESSION_MODE_ALWAYS, testvec, len);
 	v42bis(ctx, V42BIS_COMPRESSION_MODE_NEVER, testvec, len);

-- 
To view, visit https://gerrit.osmocom.org/965
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4f9adf251f5119e67ffe76baad6f1f996ac8dbad
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: dexter <pmaier at sysmocom.de>



More information about the gerrit-log mailing list