gw-bandwidth: fix potential overflow on very large input values, limit them to 100 GBit/s
PATCH 1/1 gw-bandwidth: fix potential overflow on very large input values, limit them to 100 GBit/s
--- net/batman-adv/gateway_common.c | 6 ++++++ 1 file changed, 6 insertions(+)
(quoted diff) +0 -0
b/net/batman-adv/gateway_common.c index 39cf44c..6b0f4d3 100644
net/batman-adv/gateway_common.c +0 -0
@@ -71,10 +71,16 @@ static bool batadv_parse_gw_bandwidth(struct
net_device *net_dev, char *buff,
switch (bw_unit_type) {
case BATADV_BW_UNIT_MBIT:
+ /* limit input to 100 GBit/s */
+ if (ldown > 100000)
+ ldown = 100000
*down = ldown * 10;
break;
case BATADV_BW_UNIT_KBIT:
default:
+ /* limit input to 100 GBit/s */
+ if (ldown > 100000000)
+ ldown = 100000000
*down = ldown / 100;
break;
}
Signature · 1 line
2.4.0
Sven Eckelmann
maintainerQuoted 2 lines · On Saturday 16 May 2015 22:26:43 Ruben Wisniewski wrote:
Signed-off-by Ruben Wisniewsi <[email protected]>---
Same commit message/topic problems as in the other patch. The patch also doesn't apply on master/next.
Applying: gw-bandwidth: fix potential overflow on very large input values, limit them to 100 GBit/s
fatal: corrupt patch at line 11
Patch failed at 0001 gw-bandwidth: fix potential overflow on very large input values, limit them to 100 GBit/s The copy of the patch that failed is found in:
/home/sven/tmp/batman-adv/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Quoted 14 lines
net/batman-adv/gateway_common.c | 6 ++++++ 1 file changed, 6 insertions(+)(quoted diff) +0
-0
b/net/batman-adv/gateway_common.cindex 39cf44c..6b0f4d3 100644net/batman-adv/gateway_common.c +3
-0@@ -71,10 +71,16 @@ static bool batadv_parse_gw_bandwidth(struct net_device *net_dev, char *buff, switch (bw_unit_type) {case BATADV_BW_UNIT_MBIT : +/* limit input to 100 GBit/s */ +if (ldown >100000 ) + ldown =100000
net/batman-adv/gateway_common.c static bool batadv_parse_gw_bandwidth(struct · line 86
Quoted 10 lines
net/batman-adv/gateway_common.c +3
-0*down = ldown *10 ;break ;case BATADV_BW_UNIT_KBIT :default : +/* limit input to 100 GBit/s */ +if (ldown >100000000 ) + ldown =100000000 *down = ldown /100 ;break ; }
Wouldn't it be better to reject the input and inform the user about it with an error code (EINVAL or something like that)?
The value looks a little bit randomly chosen. I've looked a little bit into the "new' TLV code and it seems store store this value inside a be32 variable when it is sent over the wire. This be32 can store 4294967295. The unit it represents seems to be in 100 Kib/s. So you would end up with a max of ~409 Tibit/s.
Maybe you can emphasize more why 100 Gibit/s is a good choice.
Btw. am I the only which gets a little bit sad about the fact that this function uses kstrtol to parse the input and then nobody check if ldown or lup is negative? ... sorry, got a little bit sidetracked.
Also, why does this change not include the upload speed parsing part of this function?
Kind regards,
Sven