| Message ID | 5568986A.7010000@iogearbox.net |
|---|---|
| State | Rejected, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
On Fri, 29 May 2015 18:48:42 +0200 Daniel Borkmann <daniel@iogearbox.net> wrote: > On 05/29/2015 06:17 PM, Guzman Mosqueda, Jose R wrote: > > Hi Daniel and Vadim > > > > Thanks for your prompt response and for the patch. > > > > Also, what about the other one? Do you think it is an issue or not? > > > > " File: tc/tc_util.c > > Function: void print_rate(char *buf, int len, __u64 rate) > > Line: ~264 > > > > In the case that user inputs a high value for rate, the "for" loop will exit in the condition meaning that variable "i" get the value of 5 which will be an invalid index for the "units" array due to that array has only 5 elements." > > > > I know a very high value is invalid but in the case that it comes directly from user, it could cause and issue, what do you think? > > Hm, this prints just the netlink dump from kernel side, but perhaps > we should just change it ... > > diff --git a/tc/tc_util.c b/tc/tc_util.c > index dc2b70f..aa6de24 100644 > --- a/tc/tc_util.c > +++ b/tc/tc_util.c > @@ -250,18 +250,19 @@ void print_rate(char *buf, int len, __u64 rate) > extern int use_iec; > unsigned long kilo = use_iec ? 1024 : 1000; > const char *str = use_iec ? "i" : ""; > - int i = 0; > static char *units[5] = {"", "K", "M", "G", "T"}; > + int i; > > rate <<= 3; /* bytes/sec -> bits/sec */ > > - for (i = 0; i < ARRAY_SIZE(units); i++) { > + for (i = 0; i < ARRAY_SIZE(units) - 1; i++) { > if (rate < kilo) > break; > if (((rate % kilo) != 0) && rate < 1000*kilo) > break; > rate /= kilo; > } > + > snprintf(buf, len, "%.0f%s%sbit", (double)rate, units[i], str); > } I don't know what thread you meant to hijack for this, but it wasn't the one about ss: cong name. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/25/2015 05:31 AM, Stephen Hemminger wrote: > On Fri, 29 May 2015 18:48:42 +0200 > Daniel Borkmann <daniel@iogearbox.net> wrote: > >> On 05/29/2015 06:17 PM, Guzman Mosqueda, Jose R wrote: >>> Hi Daniel and Vadim >>> >>> Thanks for your prompt response and for the patch. >>> >>> Also, what about the other one? Do you think it is an issue or not? >>> >>> " File: tc/tc_util.c >>> Function: void print_rate(char *buf, int len, __u64 rate) >>> Line: ~264 >>> >>> In the case that user inputs a high value for rate, the "for" loop will exit in the condition meaning that variable "i" get the value of 5 which will be an invalid index for the "units" array due to that array has only 5 elements." >>> >>> I know a very high value is invalid but in the case that it comes directly from user, it could cause and issue, what do you think? >> >> Hm, this prints just the netlink dump from kernel side, but perhaps >> we should just change it ... >> >> diff --git a/tc/tc_util.c b/tc/tc_util.c >> index dc2b70f..aa6de24 100644 >> --- a/tc/tc_util.c >> +++ b/tc/tc_util.c >> @@ -250,18 +250,19 @@ void print_rate(char *buf, int len, __u64 rate) >> extern int use_iec; >> unsigned long kilo = use_iec ? 1024 : 1000; >> const char *str = use_iec ? "i" : ""; >> - int i = 0; >> static char *units[5] = {"", "K", "M", "G", "T"}; >> + int i; >> >> rate <<= 3; /* bytes/sec -> bits/sec */ >> >> - for (i = 0; i < ARRAY_SIZE(units); i++) { >> + for (i = 0; i < ARRAY_SIZE(units) - 1; i++) { >> if (rate < kilo) >> break; >> if (((rate % kilo) != 0) && rate < 1000*kilo) >> break; >> rate /= kilo; >> } >> + >> snprintf(buf, len, "%.0f%s%sbit", (double)rate, units[i], str); >> } > > I don't know what thread you meant to hijack for this, but it wasn't the > one about ss: cong name. Jose did top-post on the first reported issue asking about the 2nd one, I think that's how we ended up here. ;) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tc/tc_util.c b/tc/tc_util.c index dc2b70f..aa6de24 100644 --- a/tc/tc_util.c +++ b/tc/tc_util.c @@ -250,18 +250,19 @@ void print_rate(char *buf, int len, __u64 rate) extern int use_iec; unsigned long kilo = use_iec ? 1024 : 1000; const char *str = use_iec ? "i" : ""; - int i = 0; static char *units[5] = {"", "K", "M", "G", "T"}; + int i; rate <<= 3; /* bytes/sec -> bits/sec */ - for (i = 0; i < ARRAY_SIZE(units); i++) { + for (i = 0; i < ARRAY_SIZE(units) - 1; i++) { if (rate < kilo) break; if (((rate % kilo) != 0) && rate < 1000*kilo) break; rate /= kilo; } + snprintf(buf, len, "%.0f%s%sbit", (double)rate, units[i], str); }