Change in osmocom-bb[master]: Revert "Move from libc random() to osmo_get_rand_id"

[Vadim Yanitskiy]

Vadim Yanitskiy has submitted this change and it was merged. ( https://gerrit.osmocom.org/10020 )

Change subject: Revert "Move from libc random() to osmo_get_rand_id"
......................................................................

Revert "Move from libc random() to osmo_get_rand_id"

It was decided to migrate to osmo_get_rand_id() and use random()
as a fall-back. But there is a critical difference between both
functions: osmo_get_rand_id() fills an input buffer with random
bytes (0x00 - 0xff), while *random() returns a value in range
between 0 and RAND_MAX.

osmo_get_rand_id() was used in a wrong way, so in some cases we
could get a negative value (how about IMEI starting from '-'?),
what isn't expected in many cases and could lead to unexpected
behaviour and segmentation faults...

This reverts commit 6d49b049ee304f1ea0e4801df61e69713b01f0f8.

Change-Id: I7b2a8a5c63cf64360a824926a2219fd7e419b1bb
---
M src/host/layer23/src/mobile/gsm322.c
M src/host/layer23/src/mobile/gsm48_mm.c
M src/host/layer23/src/mobile/gsm48_rr.c
M src/host/layer23/src/mobile/settings.c
4 files changed, 5 insertions(+), 18 deletions(-)

Approvals:
  Jenkins Builder: Verified
  Holger Freyther: Looks good to me, approved



diff --git a/src/host/layer23/src/mobile/gsm322.c b/src/host/layer23/src/mobile/gsm322.c
index ce25cd5..c3485b6 100644
--- a/src/host/layer23/src/mobile/gsm322.c
+++ b/src/host/layer23/src/mobile/gsm322.c
@@ -959,9 +959,7 @@
 			entries++;
 	}
 	while(entries) {
-		if (osmo_get_rand_id((uint8_t *) &move, sizeof(move)) != 0)
-			move = random();
-		move = move % entries;
+		move = random() % entries;
 		i = 0;
 		llist_for_each_entry(temp, &temp_list, entry) {
 			if (rxlev2dbm(temp->rxlev) > -85) {
diff --git a/src/host/layer23/src/mobile/gsm48_mm.c b/src/host/layer23/src/mobile/gsm48_mm.c
index a36e7e8..a7af1f5 100644
--- a/src/host/layer23/src/mobile/gsm48_mm.c
+++ b/src/host/layer23/src/mobile/gsm48_mm.c
@@ -30,7 +30,6 @@
 #include <osmocom/core/utils.h>
 #include <osmocom/gsm/gsm48.h>
 #include <osmocom/core/talloc.h>
-#include <osmocom/gsm/gsm_utils.h>
 
 #include <osmocom/bb/common/logging.h>
 #include <osmocom/bb/common/osmocom_data.h>
@@ -2100,9 +2099,7 @@
 			mm->t3212.timeout.tv_sec = current_time.tv_sec
 				+ (t % s->t3212);
 		} else {
-			uint32_t rand;
-			if (osmo_get_rand_id((uint8_t *) &rand, sizeof(rand)) != 0)
-				rand = random();
+			uint32_t rand = random();
 
 			LOGP(DMM, LOGL_INFO, "New T3212 while timer is not "
 				"running (value %d)\n", s->t3212);
diff --git a/src/host/layer23/src/mobile/gsm48_rr.c b/src/host/layer23/src/mobile/gsm48_rr.c
index db2cb5e..dd3fe93 100644
--- a/src/host/layer23/src/mobile/gsm48_rr.c
+++ b/src/host/layer23/src/mobile/gsm48_rr.c
@@ -71,7 +71,6 @@
 #include <osmocom/gsm/rsl.h>
 #include <osmocom/gsm/gsm48.h>
 #include <osmocom/core/bitvec.h>
-#include <osmocom/gsm/gsm_utils.h>
 
 #include <osmocom/bb/common/osmocom_data.h>
 #include <osmocom/bb/common/l1l2_interface.h>
@@ -1629,8 +1628,7 @@
 		}
 	}
 
-	if (osmo_get_rand_id((uint8_t *) &chan_req, sizeof(chan_req)) != 0)
-		chan_req = random();
+	chan_req = random();
 	chan_req &= rr->chan_req_mask;
 	chan_req |= rr->chan_req_val;
 
diff --git a/src/host/layer23/src/mobile/settings.c b/src/host/layer23/src/mobile/settings.c
index 80b0b48..7370b0a 100644
--- a/src/host/layer23/src/mobile/settings.c
+++ b/src/host/layer23/src/mobile/settings.c
@@ -23,7 +23,6 @@
 #include <errno.h>
 #include <string.h>
 #include <osmocom/core/talloc.h>
-#include <osmocom/gsm/gsm_utils.h>
 
 #include <osmocom/bb/mobile/app_mobile.h>
 #include <osmocom/bb/common/logging.h>
@@ -179,19 +178,14 @@
 {
 	int digits = set->imei_random;
 	char rand[16];
-	long rand_num;
 
 	if (digits <= 0)
 		return 0;
 	if (digits > 15)
 		digits = 15;
 
-	if (osmo_get_rand_id((uint8_t *) &rand_num, sizeof(rand_num)) != 0)
-		rand_num = random();
-	sprintf(rand, "%08ld", rand_num % 100000000);
-	if (osmo_get_rand_id((uint8_t *) &rand_num, sizeof(rand_num)) != 0)
-		rand_num = random();
-	sprintf(rand + 8, "%07ld", rand_num % 10000000);
+	sprintf(rand, "%08ld", random() % 100000000);
+	sprintf(rand + 8, "%07ld", random() % 10000000);
 
 	strcpy(set->imei + 15 - digits, rand + 15 - digits);
 	strncpy(set->imeisv, set->imei, 15);

-- 
To view, visit https://gerrit.osmocom.org/10020
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7b2a8a5c63cf64360a824926a2219fd7e419b1bb
Gerrit-Change-Number: 10020
Gerrit-PatchSet: 2
Gerrit-Owner: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180717/221b1572/attachment.htm>