On Saturday 16 May 2015 22:26:43 Ruben Wisniewski wrote:
Signed-off-by: Ruben Wisniewsi ruben@vfn-nrw.de
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".
net/batman-adv/gateway_common.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/net/batman-adv/gateway_common.c b/net/batman-adv/gateway_common.c index 39cf44c..6b0f4d3 100644 --- a/net/batman-adv/gateway_common.c +++ b/net/batman-adv/gateway_common.c @@ -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/* limit input to 100 GBit/s */if (ldown > 100000000)ldown = 100000000
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