<p>neels <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/25347">View Change</a></p><div style="white-space:pre-wrap">Approvals:
laforge: Looks good to me, but someone else must approve
neels: Looks good to me, approved
dexter: Looks good to me, but someone else must approve
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">revisit some calls of strtol(), stroul(), strtoull()<br><br>Replace some with atoi(), where the VTY has already validated correct<br>range of the argument.<br><br>Replace others with the new osmo_str_to_int() or osmo_str_to_int64()<br>functions, possibly covering more detection of invalid number strings.<br><br>Leave those strtol() callers that depend on endptr to provide the next<br>string token.<br><br>Related: SYS#5542<br>Change-Id: I0ebb06e751c28f7d1cdf328de29cd227a2449391<br>---<br>M src/ctrl/control_if.c<br>M src/gsm/gsm23003.c<br>M src/gsm/gsm23236.c<br>M src/vty/command.c<br>M src/vty/cpu_sched_vty.c<br>M src/vty/tdef_vty.c<br>M utils/osmo-aka-verify.c<br>7 files changed, 31 insertions(+), 73 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/ctrl/control_if.c b/src/ctrl/control_if.c</span><br><span>index 5fda28f..bea8496 100644</span><br><span>--- a/src/ctrl/control_if.c</span><br><span>+++ b/src/ctrl/control_if.c</span><br><span>@@ -84,20 +84,16 @@</span><br><span> * \returns 1 on success; 0 in case of error */</span><br><span> int ctrl_parse_get_num(vector vline, int i, long *num)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- char *token, *tmp;</span><br><span style="color: hsl(120, 100%, 40%);">+ char *token;</span><br><span style="color: hsl(120, 100%, 40%);">+ int64_t val;</span><br><span> </span><br><span> if (i >= vector_active(vline))</span><br><span> return 0;</span><br><span> token = vector_slot(vline, i);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- errno = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- if (token[0] == '\0')</span><br><span style="color: hsl(120, 100%, 40%);">+ if (osmo_str_to_int64(&val, token, 10, LONG_MIN, LONG_MAX))</span><br><span> return 0;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- *num = strtol(token, &tmp, 10);</span><br><span style="color: hsl(0, 100%, 40%);">- if (tmp[0] != '\0' || errno != 0)</span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(120, 100%, 40%);">+ *num = (long)val;</span><br><span> return 1;</span><br><span> }</span><br><span> </span><br><span>diff --git a/src/gsm/gsm23003.c b/src/gsm/gsm23003.c</span><br><span>index c2b3de8..f3c0123 100644</span><br><span>--- a/src/gsm/gsm23003.c</span><br><span>+++ b/src/gsm/gsm23003.c</span><br><span>@@ -487,22 +487,23 @@</span><br><span> */</span><br><span> int osmo_mnc_from_str(const char *mnc_str, uint16_t *mnc, bool *mnc_3_digits)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- long int _mnc = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ int _mnc = 0;</span><br><span> bool _mnc_3_digits = false;</span><br><span style="color: hsl(0, 100%, 40%);">- char *endptr;</span><br><span> int rc = 0;</span><br><span> </span><br><span> if (!mnc_str || !isdigit((unsigned char)mnc_str[0]) || strlen(mnc_str) > 3)</span><br><span> return -EINVAL;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- errno = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- _mnc = strtol(mnc_str, &endptr, 10);</span><br><span style="color: hsl(0, 100%, 40%);">- if (errno)</span><br><span style="color: hsl(0, 100%, 40%);">- rc = -errno;</span><br><span style="color: hsl(0, 100%, 40%);">- else if (*endptr)</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = osmo_str_to_int(&_mnc, mnc_str, 10, 0, 999);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Heed the API definition to return -EINVAL in case of surplus chars */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc == -E2BIG)</span><br><span> return -EINVAL;</span><br><span style="color: hsl(0, 100%, 40%);">- if (_mnc < 0 || _mnc > 999)</span><br><span style="color: hsl(0, 100%, 40%);">- return -ERANGE;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Heed the API definition to always return negative errno */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc > 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ return -rc;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc < 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ return rc;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> _mnc_3_digits = strlen(mnc_str) > 2;</span><br><span> </span><br><span> if (mnc)</span><br><span>diff --git a/src/gsm/gsm23236.c b/src/gsm/gsm23236.c</span><br><span>index 01d0eb3..d4a14a5 100644</span><br><span>--- a/src/gsm/gsm23236.c</span><br><span>+++ b/src/gsm/gsm23236.c</span><br><span>@@ -436,27 +436,13 @@</span><br><span> */</span><br><span> static int osmo_nri_parse(int16_t *dst, const char *str)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- char *endp;</span><br><span style="color: hsl(0, 100%, 40%);">- int64_t val;</span><br><span style="color: hsl(120, 100%, 40%);">+ int val;</span><br><span> int base = 10;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (osmo_str_startswith(str, "0x")) {</span><br><span style="color: hsl(0, 100%, 40%);">- str += 2;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (osmo_str_startswith(str, "0x"))</span><br><span> base = 16;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (!str || !str[0])</span><br><span style="color: hsl(120, 100%, 40%);">+ if (osmo_str_to_int(&val, str, base, 0, INT16_MAX))</span><br><span> return -1;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- errno = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- val = strtoull(str, &endp, base);</span><br><span style="color: hsl(0, 100%, 40%);">- if (errno || *endp != '\0')</span><br><span style="color: hsl(0, 100%, 40%);">- return -1;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (val < 0 || val > INT16_MAX)</span><br><span style="color: hsl(0, 100%, 40%);">- return -1;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- *dst = val;</span><br><span style="color: hsl(120, 100%, 40%);">+ *dst = (int16_t)val;</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span>diff --git a/src/vty/command.c b/src/vty/command.c</span><br><span>index e5e7d15..2a8942d 100644</span><br><span>--- a/src/vty/command.c</span><br><span>+++ b/src/vty/command.c</span><br><span>@@ -1349,7 +1349,6 @@</span><br><span> int colons = 0, nums = 0, double_colon = 0;</span><br><span> int mask;</span><br><span> const char *sp = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- char *endptr = NULL;</span><br><span> </span><br><span> if (str == NULL)</span><br><span> return PARTLY_MATCH;</span><br><span>@@ -1447,11 +1446,7 @@</span><br><span> if (state < STATE_MASK)</span><br><span> return PARTLY_MATCH;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- mask = strtol(str, &endptr, 10);</span><br><span style="color: hsl(0, 100%, 40%);">- if (*endptr != '\0')</span><br><span style="color: hsl(0, 100%, 40%);">- return NO_MATCH;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (mask < 0 || mask > 128)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (osmo_str_to_int(&mask, str, 10, 0, 128))</span><br><span> return NO_MATCH;</span><br><span> </span><br><span> /* I don't know why mask < 13 makes command match partly.</span><br><span>@@ -3794,16 +3789,7 @@</span><br><span> "Set number of lines on a screen\n"</span><br><span> "Number of lines on screen (0 for no pausing)\n")</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- int lines;</span><br><span style="color: hsl(0, 100%, 40%);">- char *endptr = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- lines = strtol(argv[0], &endptr, 10);</span><br><span style="color: hsl(0, 100%, 40%);">- if (lines < 0 || lines > 512 || *endptr != '\0') {</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, "length is malformed%s", VTY_NEWLINE);</span><br><span style="color: hsl(0, 100%, 40%);">- return CMD_WARNING;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- vty->lines = lines;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(120, 100%, 40%);">+ vty->lines = atoi(argv[0]);</span><br><span> return CMD_SUCCESS;</span><br><span> }</span><br><span> </span><br><span>@@ -3822,16 +3808,7 @@</span><br><span> "System wide terminal length configuration\n"</span><br><span> "Number of lines of VTY (0 means no line control)\n")</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- int lines;</span><br><span style="color: hsl(0, 100%, 40%);">- char *endptr = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- lines = strtol(argv[0], &endptr, 10);</span><br><span style="color: hsl(0, 100%, 40%);">- if (lines < 0 || lines > 512 || *endptr != '\0') {</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, "length is malformed%s", VTY_NEWLINE);</span><br><span style="color: hsl(0, 100%, 40%);">- return CMD_WARNING;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- host.lines = lines;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(120, 100%, 40%);">+ host.lines = atoi(argv[0]);</span><br><span> return CMD_SUCCESS;</span><br><span> }</span><br><span> </span><br><span>diff --git a/src/vty/cpu_sched_vty.c b/src/vty/cpu_sched_vty.c</span><br><span>index 4ccc627..0b4b249 100644</span><br><span>--- a/src/vty/cpu_sched_vty.c</span><br><span>+++ b/src/vty/cpu_sched_vty.c</span><br><span>@@ -276,7 +276,6 @@</span><br><span> static enum sched_vty_thread_id procname2pid(pid_t *res_pid, const char *str, bool applynow)</span><br><span> {</span><br><span> size_t i, len;</span><br><span style="color: hsl(0, 100%, 40%);">- char *end;</span><br><span> bool is_pid = true;</span><br><span> </span><br><span> if (strcmp(str, "all") == 0) {</span><br><span>@@ -297,12 +296,12 @@</span><br><span> }</span><br><span> }</span><br><span> if (is_pid) {</span><br><span style="color: hsl(0, 100%, 40%);">- errno = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- *res_pid = strtoul(str, &end, 0);</span><br><span style="color: hsl(0, 100%, 40%);">- if ((errno == ERANGE && *res_pid == ULONG_MAX) || (errno && !*res_pid) ||</span><br><span style="color: hsl(0, 100%, 40%);">- str == end) {</span><br><span style="color: hsl(120, 100%, 40%);">+ int64_t val;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (osmo_str_to_int64(&val, str, 0, 0, INT64_MAX))</span><br><span> return SCHED_VTY_THREAD_UNKNOWN;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ *res_pid = (pid_t)val;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (*res_pid != val)</span><br><span style="color: hsl(120, 100%, 40%);">+ return SCHED_VTY_THREAD_UNKNOWN;</span><br><span> if (!applynow || proc_tid_exists(*res_pid))</span><br><span> return SCHED_VTY_THREAD_ID;</span><br><span> else</span><br><span>diff --git a/src/vty/tdef_vty.c b/src/vty/tdef_vty.c</span><br><span>index 0556d8c..09459f1 100644</span><br><span>--- a/src/vty/tdef_vty.c</span><br><span>+++ b/src/vty/tdef_vty.c</span><br><span>@@ -50,10 +50,9 @@</span><br><span> */</span><br><span> struct osmo_tdef *osmo_tdef_vty_parse_T_arg(struct vty *vty, struct osmo_tdef *tdefs, const char *T_str)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- long l;</span><br><span style="color: hsl(120, 100%, 40%);">+ int l;</span><br><span> int T;</span><br><span> struct osmo_tdef *t;</span><br><span style="color: hsl(0, 100%, 40%);">- char *endptr;</span><br><span> const char *T_nr_str;</span><br><span> int sign = 1;</span><br><span> </span><br><span>@@ -77,9 +76,7 @@</span><br><span> return NULL;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- errno = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- l = strtol(T_nr_str, &endptr, 10);</span><br><span style="color: hsl(0, 100%, 40%);">- if (errno || *endptr || l > INT_MAX || l < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (osmo_str_to_int(&l, T_nr_str, 10, 0, INT_MAX)) {</span><br><span> vty_out(vty, "%% Invalid T timer argument (should be 'T1234' or 'X1234'): '%s'%s", T_str, VTY_NEWLINE);</span><br><span> return NULL;</span><br><span> }</span><br><span>diff --git a/utils/osmo-aka-verify.c b/utils/osmo-aka-verify.c</span><br><span>index 086add5..bbc65ef 100644</span><br><span>--- a/utils/osmo-aka-verify.c</span><br><span>+++ b/utils/osmo-aka-verify.c</span><br><span>@@ -123,6 +123,7 @@</span><br><span> bool opc_is_set = false;</span><br><span> bool amf_is_set = false;</span><br><span> bool opc_is_op = false;</span><br><span style="color: hsl(120, 100%, 40%);">+ int64_t val64;</span><br><span> </span><br><span> while (1) {</span><br><span> int c;</span><br><span>@@ -169,7 +170,8 @@</span><br><span> amf_is_set = true;</span><br><span> break;</span><br><span> case 's':</span><br><span style="color: hsl(0, 100%, 40%);">- g_sqn = strtoull(optarg, 0, 10);</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = osmo_str_to_int64(&val64, optarg, 10, 0, INT64_MAX);</span><br><span style="color: hsl(120, 100%, 40%);">+ g_sqn = (unsigned long long)val64;</span><br><span> sqn_is_set = true;</span><br><span> break;</span><br><span> case 'r':</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/25347">change 25347</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/libosmocore/+/25347"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I0ebb06e751c28f7d1cdf328de29cd227a2449391 </div>
<div style="display:none"> Gerrit-Change-Number: 25347 </div>
<div style="display:none"> Gerrit-PatchSet: 10 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>