[PATCH] openbsc[master]: Fix BTS features length check

2017-06-13 Thread Max

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

Fix BTS features length check

While fixing potentially incorrect memory access, the check for maximum
number of supported BTS features was incorrectly adjusted instead of
feature vectore length check next to it. Fix this by adjusting checks
properly and adding comments to avoid future confusion.

The error was introduced in a60bb3dd28ce9e3720f8ee1b262893f3e233e2e6.

Change-Id: I06d2498d730624d5da535f6add6fa98d004714ae
---
M openbsc/src/libbsc/abis_nm.c
1 file changed, 4 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/00/2900/1

diff --git a/openbsc/src/libbsc/abis_nm.c b/openbsc/src/libbsc/abis_nm.c
index 1715688..b0f3428 100644
--- a/openbsc/src/libbsc/abis_nm.c
+++ b/openbsc/src/libbsc/abis_nm.c
@@ -484,13 +484,13 @@
if (TLVP_PRES_LEN(&tp, NM_ATT_MANUF_ID, 2)) {
m_id_len = TLVP_LEN(&tp, NM_ATT_MANUF_ID);
 
-   if (m_id_len > MAX_BTS_FEATURES/8 + 1) {
+   /* log potential BTS feature vector overflow */
+   if (m_id_len > sizeof(bts->_features_data))
LOGP(DNM, LOGL_NOTICE, "BTS%u Get Attributes Response: 
feature vector is truncated to %u bytes\n",
 bts->nr, MAX_BTS_FEATURES/8);
-   m_id_len = MAX_BTS_FEATURES/8;
-   }
 
-   if (m_id_len > sizeof(bts->_features_data))
+   /* check that max. expected BTS attribute is above given 
feature vector length */
+   if (m_id_len > _NUM_BTS_FEAT/8 + 1)
LOGP(DNM, LOGL_NOTICE, "BTS%u Get Attributes Response: 
reported unexpectedly long (%u bytes) "
 "feature vector - most likely it was compiled 
against newer BSC headers. "
 "Consider upgrading your BSC to later version.\n",

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I06d2498d730624d5da535f6add6fa98d004714ae
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max 


[PATCH] openbsc[master]: Fix BTS features length check

2017-06-19 Thread Max
Hello Neels Hofmeyr, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/2900

to look at the new patch set (#2).

Fix BTS features length check

While fixing potentially incorrect memory access, the check for maximum
number of supported BTS features was incorrectly adjusted instead of
feature vectore length check next to it. Fix this by adjusting checks
properly and adding comments to avoid future confusion.

The error was introduced in a60bb3dd28ce9e3720f8ee1b262893f3e233e2e6.

Change-Id: I06d2498d730624d5da535f6add6fa98d004714ae
---
M openbsc/src/libbsc/abis_nm.c
1 file changed, 4 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/00/2900/2

diff --git a/openbsc/src/libbsc/abis_nm.c b/openbsc/src/libbsc/abis_nm.c
index 1715688..019d039 100644
--- a/openbsc/src/libbsc/abis_nm.c
+++ b/openbsc/src/libbsc/abis_nm.c
@@ -484,13 +484,13 @@
if (TLVP_PRES_LEN(&tp, NM_ATT_MANUF_ID, 2)) {
m_id_len = TLVP_LEN(&tp, NM_ATT_MANUF_ID);
 
-   if (m_id_len > MAX_BTS_FEATURES/8 + 1) {
+   /* log potential BTS feature vector overflow */
+   if (m_id_len > sizeof(bts->_features_data))
LOGP(DNM, LOGL_NOTICE, "BTS%u Get Attributes Response: 
feature vector is truncated to %u bytes\n",
 bts->nr, MAX_BTS_FEATURES/8);
-   m_id_len = MAX_BTS_FEATURES/8;
-   }
 
-   if (m_id_len > sizeof(bts->_features_data))
+   /* check that max. expected BTS attribute is above given 
feature vector length */
+   if (m_id_len > OSMO_BYTES_FOR_BITS(_NUM_BTS_FEAT))
LOGP(DNM, LOGL_NOTICE, "BTS%u Get Attributes Response: 
reported unexpectedly long (%u bytes) "
 "feature vector - most likely it was compiled 
against newer BSC headers. "
 "Consider upgrading your BSC to later version.\n",

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06d2498d730624d5da535f6add6fa98d004714ae
Gerrit-PatchSet: 2
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr