When a numerical command line param value is out of range or contains garbage
(both tgtd and tgtadm), this remains unchecked in many cases.
In some of them 0 value will be used, which may lead to unpredicted or
"silently" faulty behavior.
This fix defines a new function str_to_uit64() which uses strtoull() and
detects its errors properly, by checking 1) errno (range errors) and
2) comparing endptr to the original argument (nun-numerical garbage);
must check both.
str_to_uit64() is declared static twice, in tgtd.c and tgtadm.c, as
tgtadm does not uses util.c, where the natural place of this function
would be otherwise.
It returns the converted integer value directly, while error is returned
through a pointer argument. Although the opposite arrangement may seem
intuitive, this one allows free casting of the returned value to various
types. Return type uint64 is choosen to cover all possible cases.
min and max arguments support range checking. To use the natural integer
ranges, use ULLONG_MAX, ULONG_MAX, USHRT_MAX etc.
Signed-off-by: Alexander Nezhinsky <alexandern@xxxxxxxxxxxx>
---
usr/tgtadm.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
usr/tgtd.c | 47 +++++++++++++++++++++++++++++++++++-
2 files changed, 109 insertions(+), 11 deletions(-)
diff --git a/usr/tgtadm.c b/usr/tgtadm.c
index 28ed754..d2b9649 100644
--- a/usr/tgtadm.c
+++ b/usr/tgtadm.c
@@ -30,6 +30,7 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <limits.h>
#include <sys/socket.h>
#include <sys/stat.h>
@@ -308,6 +309,34 @@ static int filter(const struct dirent *dir)
return strcmp(dir->d_name, ".") && strcmp(dir->d_name, "..");
}
+/* Integer value is returned directly, while error is returned through
+ a pointer argument. Although the opposite arrangement may seem intuitive,
+ this one allows free casting of the returned value to various types.
+ Return type uint64 is choosen to cover all possible cases.
+ min and max arguments support range checking. To use the natural integer
+ ranges, use ULLONG_MAX, ULONG_MAX, USHRT_MAX etc.
+*/
+static uint64_t str_to_uint64(char *str,
+ uint64_t min, uint64_t max,
+ int *err_ptr)
+{
+ uint64_t val = 0;
+ int err = 0;
+ char *endptr;
+
+ errno = 0;
+ val = (uint64_t) strtoull(str, &endptr, 0); /* decimal or hex with 0x */
+ if (errno != 0 || endptr == str)
+ err = -EINVAL;
+
+ if (val < min || val > max)
+ err = -ERANGE;
+
+ if (err_ptr)
+ *err_ptr = err;
+ return val;
+}
+
static int bus_to_host(char *bus)
{
int i, nr, host = -1;
@@ -471,20 +500,34 @@ int main(int argc, char **argv)
mode = str_to_mode(optarg);
break;
case 't':
- tid = strtol(optarg, NULL, 10);
- if (errno == EINVAL) {
- eprintf("invalid tid '%s'\n", optarg);
- exit(EINVAL);
+ tid = (int) str_to_uint64(optarg, 0, INT_MAX, &rc);
+ if (rc) {
+ eprintf("Invalid tid: '%s'\n", optarg);
+ usage(1);
}
break;
case 's':
- sid = strtoull(optarg, NULL, 10);
+ sid = str_to_uint64(optarg, 0, ULLONG_MAX, &rc);
+ if (rc) {
+ eprintf("Invalid sid: '%s'\n", optarg);
+ usage(1);
+ }
break;
case 'c':
- cid = strtoul(optarg, NULL, 10);
+ cid = (uint32_t) str_to_uint64(optarg,
+ 0, ULONG_MAX,
+ &rc);
+ if (rc) {
+ eprintf("Invalid cid: '%s'\n", optarg);
+ usage(1);
+ }
break;
case 'l':
- lun = strtoull(optarg, NULL, 10);
+ lun = str_to_uint64(optarg, 0, ULLONG_MAX, &rc);
+ if (rc) {
+ eprintf("Invalid lun: '%s'\n", optarg);
+ usage(1);
+ }
break;
case 'P':
targetOps = optarg;
@@ -514,7 +557,13 @@ int main(int argc, char **argv)
hostno = bus_to_host(optarg);
break;
case 'H':
- hostno = strtol(optarg, NULL, 10);
+ hostno = (uint32_t) str_to_uint64(optarg,
+ 0, ULONG_MAX,
+ &rc);
+ if (rc) {
+ eprintf("Invalid hostno: '%s'\n", optarg);
+ usage(1);
+ }
break;
case 'E':
bstype = optarg;
@@ -526,7 +575,13 @@ int main(int argc, char **argv)
ac_dir = ACCOUNT_TYPE_OUTGOING;
break;
case 'C':
- control_port = strtol(optarg, NULL, 10);
+ control_port = (short int) str_to_uint64(optarg,
+ 0, SHRT_MAX,
+ &rc);
+ if (rc) {
+ eprintf("Invalid control port: '%s'\n", optarg);
+ usage(1);
+ }
break;
case 'd':
debug = 1;
diff --git a/usr/tgtd.c b/usr/tgtd.c
index 8fbefb1..7231de6 100644
--- a/usr/tgtd.c
+++ b/usr/tgtd.c
@@ -30,6 +30,7 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <limits.h>
#include <sys/resource.h>
#include <sys/epoll.h>
@@ -394,6 +395,34 @@ static void lld_exit(void)
}
}
+/* Integer value is returned directly, while error is returned through
+ a pointer argument. Although the opposite arrangement may seem intuitive,
+ this one allows free casting of the returned value to various types.
+ Return type uint64 is choosen to cover all possible cases.
+ min and max arguments support range checking. To use the natural integer
+ ranges, use ULLONG_MAX, ULONG_MAX, USHRT_MAX etc.
+*/
+static uint64_t str_to_uint64(char *str,
+ uint64_t min, uint64_t max,
+ int *err_ptr)
+{
+ uint64_t val = 0;
+ int err = 0;
+ char *endptr;
+
+ errno = 0;
+ val = (uint64_t) strtoull(str, &endptr, 0); /* decimal or hex with 0x */
+ if (errno != 0 || endptr == str)
+ err = -EINVAL;
+
+ if (val < min || val > max)
+ err = -ERANGE;
+
+ if (err_ptr)
+ *err_ptr = err;
+ return val;
+}
+
struct tgt_param {
int (*parse_func)(char *);
char *name;
@@ -463,10 +492,24 @@ int main(int argc, char **argv)
is_daemon = 0;
break;
case 'C':
- control_port = atoi(optarg);
+ control_port = (short int) str_to_uint64(optarg,
+ 0, SHRT_MAX,
+ &ret);
+ if (ret) {
+ fprintf(stderr, "Invalid control port: '%s'\n",
+ optarg);
+ usage(1);
+ }
break;
case 'd':
- is_debug = atoi(optarg);
+ is_debug = (int) str_to_uint64(optarg,
+ 0, INT_MAX,
+ &ret);
+ if (ret) {
+ fprintf(stderr, "Invalid debug port: '%s'\n",
+ optarg);
+ usage(1);
+ }
break;
case 'v':
exit(0);
--
1.6.5
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
