I've got a fix that I'm testing. I'll forward once I'm happy
Sent from my iPhone
On 20/06/2010, at 12:01, FUJITA Tomonori
<fujita.tomonori@xxxxxxxxxxxxx> wrote:
On Thu, 17 Jun 2010 16:19:43 +0300 (IDT)
Alexander Nezhinsky <alexandern@xxxxxxxxxxxx> wrote:
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>
I'm for the strict checking however two comments.
- casting a returned value looks ugly.
- duplicating the same function isn't nice.
How about something like this?
diff --git a/usr/tgtadm.c b/usr/tgtadm.c
index 28ed754..2282c86 100644
--- a/usr/tgtadm.c
+++ b/usr/tgtadm.c
@@ -471,8 +471,8 @@ int main(int argc, char **argv)
mode = str_to_mode(optarg);
break;
case 't':
- tid = strtol(optarg, NULL, 10);
- if (errno == EINVAL) {
+ tid = str_to_val(optarg, int, 0, INT_MAX, &rc);
+ if (rc == EINVAL) {
eprintf("invalid tid '%s'\n", optarg);
exit(EINVAL);
}
diff --git a/usr/util.h b/usr/util.h
index 6e1fd6f..fc406b1 100644
--- a/usr/util.h
+++ b/usr/util.h
@@ -156,4 +156,17 @@ struct signalfd_siginfo {
};
#endif
+#define str_to_val(str, type, min, max, err_ptr) \
+({ \
+ char *ptr; \
+ int ret = 0; \
+ typeof(type) v = strtoull(str, &ptr, 0); \
+ if (errno || ptr == str) \
+ ret = EINVAL; \
+ if (v < min|| v > max) \
+ ret = ERANGE; \
+ *err_ptr = ret; \
+ v; \
+}) \
+
#endif
--
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
--
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
