[PATCH 5/8] Improve str_to_int() There is a potential problem when a user-supplied value overflows the accepting variable; to cope with this the string is converted to a 64-bit value first then assigned to the variable, then the macro checks if they represent the same value. Thus the natural type boundaries are automatically guarded. This means that there is no need to use type's range limits (e.g. 0 and USHRT_MAX for unsigned short). Thus the range limit parameters are removed from str_to_int() -- it's enough for the (most frequent) cases when any value fitting a certain integer type is admissible. When the min/max values are relevant they can be passed to one of the new macros str_to_int_gt, str_to_int_ge, str_to_int_lt, str_to_int_range, requiring that the int value should be greater than, greater or equal, less than, or within a range, correspondingly.

[nezhinsky@xxxxxxxxx]

From: Alexander Nezhinsky <alexandern@xxxxxxxxxxxx>


Signed-off-by: Alexander Nezhinsky <alexandern@xxxxxxxxxxxx>
---
 usr/tgtadm.c |   12 ++++++------
 usr/tgtd.c   |    6 +++---
 usr/util.h   |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 59 insertions(+), 17 deletions(-)

diff --git a/usr/tgtadm.c b/usr/tgtadm.c
index 0d8f5c0..92f2dc7 100644
--- a/usr/tgtadm.c
+++ b/usr/tgtadm.c
@@ -501,22 +501,22 @@ int main(int argc, char **argv)
 			mode = str_to_mode(optarg);
 			break;
 		case 't':
-			rc = str_to_int(optarg, tid, 0, INT_MAX);
+			rc = str_to_int_ge(optarg, tid, 0);
 			if (rc)
 				bad_optarg(rc, ch, optarg);
 			break;
 		case 's':
-			rc = str_to_int(optarg, sid, 0, ULLONG_MAX);
+			rc = str_to_int(optarg, sid);
 			if (rc)
 				bad_optarg(rc, ch, optarg);
 			break;
 		case 'c':
-			rc = str_to_int(optarg, cid, 0, UINT_MAX);
+			rc = str_to_int(optarg, cid);
 			if (rc)
 				bad_optarg(rc, ch, optarg);
 			break;
 		case 'l':
-			rc = str_to_int(optarg, lun, 0, ULLONG_MAX);
+			rc = str_to_int(optarg, lun);
 			if (rc)
 				bad_optarg(rc, ch, optarg);
 			break;
@@ -554,7 +554,7 @@ int main(int argc, char **argv)
 			hostno = bus_to_host(optarg);
 			break;
 		case 'H':
-			rc = str_to_int(optarg, hostno, 0, UINT_MAX);
+			rc = str_to_int_ge(optarg, hostno, 0);
 			if (rc)
 				bad_optarg(rc, ch, optarg);
 			break;
@@ -577,7 +577,7 @@ int main(int argc, char **argv)
 			ac_dir = ACCOUNT_TYPE_OUTGOING;
 			break;
 		case 'C':
-			rc = str_to_int(optarg, control_port, 0, USHRT_MAX);
+			rc = str_to_int_gt(optarg, control_port, 0);
 			if (rc)
 				bad_optarg(rc, ch, optarg);
 			break;
diff --git a/usr/tgtd.c b/usr/tgtd.c
index b5d7ec3..4ec6f23 100644
--- a/usr/tgtd.c
+++ b/usr/tgtd.c
@@ -511,17 +511,17 @@ int main(int argc, char **argv)
 			is_daemon = 0;
 			break;
 		case 'C':
-			ret = str_to_int(optarg, control_port, 0, USHRT_MAX);
+			ret = str_to_int_gt(optarg, control_port, 0);
 			if (ret)
 				bad_optarg(ret, ch, optarg);
 			break;
 		case 't':
-			ret = str_to_int(optarg, nr_iothreads, 0, USHRT_MAX);
+			ret = str_to_int_gt(optarg, nr_iothreads, 0);
 			if (ret)
 				bad_optarg(ret, ch, optarg);
 			break;
 		case 'd':
-			ret = str_to_int(optarg, is_debug, 0, 1);
+			ret = str_to_int_range(optarg, is_debug, 0, 1);
 			if (ret)
 				bad_optarg(ret, ch, optarg);
 			break;
diff --git a/usr/util.h b/usr/util.h
index 4bf157d..aefb20c 100644
--- a/usr/util.h
+++ b/usr/util.h
@@ -133,18 +133,60 @@ struct signalfd_siginfo {
 };
 #endif
 
-#define str_to_int(str, val, minv, maxv)		\
-({							\
-	char *ptr;					\
-	int ret = 0;					\
-	val = (typeof(val)) strtoull(str, &ptr, 0);	\
-	if (errno || ptr == str) 			\
-		ret = EINVAL;				\
-	else if (val < minv || val > maxv)		\
+/* convert string to integer, check for validity of the string numeric format
+ * and the natural boundaries of the integer value type (first get a 64-bit
+ * value and check that it fits the range of the destination integer).
+ */
+#define str_to_int(str, val)    			\
+({      						\
+	int ret = 0;    				\
+	char *ptr;      				\
+	unsigned long long ull_val;     		\
+	ull_val = strtoull(str, &ptr, 0);       	\
+	val = (typeof(val)) ull_val;    		\
+	if (errno || ptr == str)			\
+		ret = EINVAL;   			\
+	else if (val != ull_val)			\
+		ret = ERANGE;   			\
+	ret;						\
+})
+
+/* convert to int and check: strictly greater than */
+#define str_to_int_gt(str, val, minv)   		\
+({      						\
+	int ret = str_to_int(str, val); 		\
+	if (!ret && (val < minv))       		\
+		ret = ERANGE;   			\
+	ret;						\
+})
+
+/* convert and check: greater than or equal  */
+#define str_to_int_ge(str, val, minv)   		\
+({      						\
+	int ret = str_to_int(str, val); 		\
+	if (!ret && (val < minv))       		\
+		ret = ERANGE;   			\
+	ret;    					\
+})
+
+/* convert and check: strictly less than  */
+#define str_to_int_lt(str, val, maxv)   		\
+({      						\
+	int ret = str_to_int(str, val); 		\
+	if (!ret && (val > maxv))       		\
 		ret = ERANGE;				\
 	ret;						\
 })
 
+/* convert and check: range, ends inclusive  */
+#define str_to_int_range(str, val, minv, maxv)  	\
+({      						\
+	int ret = str_to_int(str, val); 		\
+	if (!ret && (val < minv || val > maxv)) 	\
+		ret = ERANGE;   			\
+	ret;						\
+})
+
 struct concat_buf {
 	FILE *streamf;
 	int err;
-- 
1.7.9.6

--
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