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".
> 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
> *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
signature.asc
Description: This is a digitally signed message part.
