* [B.A.T.M.A.N.] [PATCH v2 1/4] batman-adv: Return EINVAL on invalid gw_bandwidth change
@ 2015-06-21 12:42 Sven Eckelmann
2015-06-21 12:42 ` [B.A.T.M.A.N.] [PATCH v2 2/4] batman-adv: Backport kstrtou64 for Linux < 2.6.39 Sven Eckelmann
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Sven Eckelmann @ 2015-06-21 12:42 UTC (permalink / raw)
To: b.a.t.m.a.n
Invalid speed settings by the user are currently acknowledged as correct
but not stored. Instead the return of the store operation of the file
"gw_bandwidth" should indicate that the given value is not acceptable.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
v2:
* rebased on current master
* add missing header linux/errno.h
net/batman-adv/gateway_common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/gateway_common.c b/net/batman-adv/gateway_common.c
index c50931c..6b930a6 100644
--- a/net/batman-adv/gateway_common.c
+++ b/net/batman-adv/gateway_common.c
@@ -19,6 +19,7 @@
#include "main.h"
#include <linux/atomic.h>
+#include <linux/errno.h>
#include <linux/byteorder/generic.h>
#include <linux/kernel.h>
#include <linux/netdevice.h>
@@ -160,7 +161,7 @@ ssize_t batadv_gw_bandwidth_set(struct net_device *net_dev, char *buff,
ret = batadv_parse_gw_bandwidth(net_dev, buff, &down_new, &up_new);
if (!ret)
- goto end;
+ return -EINVAL;
if (!down_new)
down_new = 1;
@@ -184,7 +185,6 @@ ssize_t batadv_gw_bandwidth_set(struct net_device *net_dev, char *buff,
atomic_set(&bat_priv->gw.bandwidth_up, up_new);
batadv_gw_tvlv_container_update(bat_priv);
-end:
return count;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [B.A.T.M.A.N.] [PATCH v2 2/4] batman-adv: Backport kstrtou64 for Linux < 2.6.39
2015-06-21 12:42 [B.A.T.M.A.N.] [PATCH v2 1/4] batman-adv: Return EINVAL on invalid gw_bandwidth change Sven Eckelmann
@ 2015-06-21 12:42 ` Sven Eckelmann
2015-07-10 8:01 ` Marek Lindner
2015-06-21 12:42 ` [B.A.T.M.A.N.] [PATCH v2 3/4] batman-adv: Define (u|s)(8|16|32|64) limits for Linux < 3.14 Sven Eckelmann
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Sven Eckelmann @ 2015-06-21 12:42 UTC (permalink / raw)
To: b.a.t.m.a.n
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
v2:
* rebased on current master
compat-include/linux/kernel.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/compat-include/linux/kernel.h b/compat-include/linux/kernel.h
index 2015f7f..663f9e9 100644
--- a/compat-include/linux/kernel.h
+++ b/compat-include/linux/kernel.h
@@ -36,6 +36,18 @@
_r = -ERANGE;\
_r;\
})
+
+#define kstrtou64(cp, base, v)\
+({\
+ unsigned long long _v;\
+ int _r;\
+ _r = strict_strtoull(cp, base, &_v);\
+ *(v) = (uint64_t)_v;\
+ if ((unsigned long long)*(v) != _v)\
+ _r = -ERANGE;\
+ _r;\
+})
+
#define kstrtoul strict_strtoul
#define kstrtol strict_strtol
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [B.A.T.M.A.N.] [PATCH v2 3/4] batman-adv: Define (u|s)(8|16|32|64) limits for Linux < 3.14
2015-06-21 12:42 [B.A.T.M.A.N.] [PATCH v2 1/4] batman-adv: Return EINVAL on invalid gw_bandwidth change Sven Eckelmann
2015-06-21 12:42 ` [B.A.T.M.A.N.] [PATCH v2 2/4] batman-adv: Backport kstrtou64 for Linux < 2.6.39 Sven Eckelmann
@ 2015-06-21 12:42 ` Sven Eckelmann
2015-07-10 8:02 ` Marek Lindner
2015-06-21 12:42 ` [B.A.T.M.A.N.] [PATCH v2 4/4] batman-adv: Fix gw_bandwidth calculation on 32 bit systems Sven Eckelmann
2015-06-28 14:40 ` [B.A.T.M.A.N.] [PATCH v2 1/4] batman-adv: Return EINVAL on invalid gw_bandwidth change Marek Lindner
3 siblings, 1 reply; 11+ messages in thread
From: Sven Eckelmann @ 2015-06-21 12:42 UTC (permalink / raw)
To: b.a.t.m.a.n
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
v2:
* rebased on current master
compat-include/linux/kernel.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/compat-include/linux/kernel.h b/compat-include/linux/kernel.h
index 663f9e9..c39cbe8 100644
--- a/compat-include/linux/kernel.h
+++ b/compat-include/linux/kernel.h
@@ -53,4 +53,21 @@
#endif /* < KERNEL_VERSION(2, 6, 39) */
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 0)
+
+#define U8_MAX ((u8)~0U)
+#define S8_MAX ((s8)(U8_MAX >> 1))
+#define S8_MIN ((s8)(-S8_MAX - 1))
+#define U16_MAX ((u16)~0U)
+#define S16_MAX ((s16)(U16_MAX >> 1))
+#define S16_MIN ((s16)(-S16_MAX - 1))
+#define U32_MAX ((u32)~0U)
+#define S32_MAX ((s32)(U32_MAX >> 1))
+#define S32_MIN ((s32)(-S32_MAX - 1))
+#define U64_MAX ((u64)~0ULL)
+#define S64_MAX ((s64)(U64_MAX >> 1))
+#define S64_MIN ((s64)(-S64_MAX - 1))
+
+#endif /* < KERNEL_VERSION(3, 14, 0) */
+
#endif /* _NET_BATMAN_ADV_COMPAT_LINUX_KERNEL_H_ */
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [B.A.T.M.A.N.] [PATCH v2 4/4] batman-adv: Fix gw_bandwidth calculation on 32 bit systems
2015-06-21 12:42 [B.A.T.M.A.N.] [PATCH v2 1/4] batman-adv: Return EINVAL on invalid gw_bandwidth change Sven Eckelmann
2015-06-21 12:42 ` [B.A.T.M.A.N.] [PATCH v2 2/4] batman-adv: Backport kstrtou64 for Linux < 2.6.39 Sven Eckelmann
2015-06-21 12:42 ` [B.A.T.M.A.N.] [PATCH v2 3/4] batman-adv: Define (u|s)(8|16|32|64) limits for Linux < 3.14 Sven Eckelmann
@ 2015-06-21 12:42 ` Sven Eckelmann
2015-06-29 9:47 ` Antonio Quartulli
2015-07-10 8:04 ` Marek Lindner
2015-06-28 14:40 ` [B.A.T.M.A.N.] [PATCH v2 1/4] batman-adv: Return EINVAL on invalid gw_bandwidth change Marek Lindner
3 siblings, 2 replies; 11+ messages in thread
From: Sven Eckelmann @ 2015-06-21 12:42 UTC (permalink / raw)
To: b.a.t.m.a.n
The TVLV for the gw_bandwidth stores everything as u32. But the
gw_bandwidth reads the signed long which limits the maximum value to
(2 ** 31 - 1) on systems with 4 byte long. Also the input value is always
converted from either Mibit/s or Kibit/s to 100Kibit/s. This reduces the
values even further when the user sets it via the default unit Kibit/s. It
may even cause an integer overflow and end up with a value the user never
intended.
Instead read the values as u64, check for possible overflows, do the unit
adjustments and then reduce the size to u32.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
v2:
* rebased on current master
net/batman-adv/gateway_common.c | 49 +++++++++++++++++++++++++++++++++++------
1 file changed, 42 insertions(+), 7 deletions(-)
diff --git a/net/batman-adv/gateway_common.c b/net/batman-adv/gateway_common.c
index 6b930a6..058b957 100644
--- a/net/batman-adv/gateway_common.c
+++ b/net/batman-adv/gateway_common.c
@@ -18,6 +18,7 @@
#include "gateway_common.h"
#include "main.h"
+#include <asm/div64.h>
#include <linux/atomic.h>
#include <linux/errno.h>
#include <linux/byteorder/generic.h>
@@ -44,7 +45,7 @@ static bool batadv_parse_gw_bandwidth(struct net_device *net_dev, char *buff,
{
enum batadv_bandwidth_units bw_unit_type = BATADV_BW_UNIT_KBIT;
char *slash_ptr, *tmp_ptr;
- long ldown, lup;
+ u64 ldown, lup;
int ret;
slash_ptr = strchr(buff, '/');
@@ -62,7 +63,7 @@ static bool batadv_parse_gw_bandwidth(struct net_device *net_dev, char *buff,
*tmp_ptr = '\0';
}
- ret = kstrtol(buff, 10, &ldown);
+ ret = kstrtou64(buff, 10, &ldown);
if (ret) {
batadv_err(net_dev,
"Download speed of gateway mode invalid: %s\n",
@@ -72,14 +73,31 @@ static bool batadv_parse_gw_bandwidth(struct net_device *net_dev, char *buff,
switch (bw_unit_type) {
case BATADV_BW_UNIT_MBIT:
- *down = ldown * 10;
+ /* prevent overflow */
+ if (U64_MAX / 10 < ldown) {
+ batadv_err(net_dev,
+ "Download speed of gateway mode too large: %s\n",
+ buff);
+ return false;
+ }
+
+ ldown *= 10;
break;
case BATADV_BW_UNIT_KBIT:
default:
- *down = ldown / 100;
+ do_div(ldown, 100);
break;
}
+ if (U32_MAX < ldown) {
+ batadv_err(net_dev,
+ "Download speed of gateway mode too large: %s\n",
+ buff);
+ return false;
+ }
+
+ *down = ldown;
+
/* we also got some upload info */
if (slash_ptr) {
bw_unit_type = BATADV_BW_UNIT_KBIT;
@@ -95,7 +113,7 @@ static bool batadv_parse_gw_bandwidth(struct net_device *net_dev, char *buff,
*tmp_ptr = '\0';
}
- ret = kstrtol(slash_ptr + 1, 10, &lup);
+ ret = kstrtou64(slash_ptr + 1, 10, &lup);
if (ret) {
batadv_err(net_dev,
"Upload speed of gateway mode invalid: %s\n",
@@ -105,13 +123,30 @@ static bool batadv_parse_gw_bandwidth(struct net_device *net_dev, char *buff,
switch (bw_unit_type) {
case BATADV_BW_UNIT_MBIT:
- *up = lup * 10;
+ /* prevent overflow */
+ if (U64_MAX / 10 < lup) {
+ batadv_err(net_dev,
+ "Upload speed of gateway mode too large: %s\n",
+ slash_ptr + 1);
+ return false;
+ }
+
+ lup *= 10;
break;
case BATADV_BW_UNIT_KBIT:
default:
- *up = lup / 100;
+ do_div(lup, 100);
break;
}
+
+ if (U32_MAX < lup) {
+ batadv_err(net_dev,
+ "Upload speed of gateway mode too large: %s\n",
+ slash_ptr + 1);
+ return false;
+ }
+
+ *up = lup;
}
return true;
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH v2 1/4] batman-adv: Return EINVAL on invalid gw_bandwidth change
2015-06-21 12:42 [B.A.T.M.A.N.] [PATCH v2 1/4] batman-adv: Return EINVAL on invalid gw_bandwidth change Sven Eckelmann
` (2 preceding siblings ...)
2015-06-21 12:42 ` [B.A.T.M.A.N.] [PATCH v2 4/4] batman-adv: Fix gw_bandwidth calculation on 32 bit systems Sven Eckelmann
@ 2015-06-28 14:40 ` Marek Lindner
3 siblings, 0 replies; 11+ messages in thread
From: Marek Lindner @ 2015-06-28 14:40 UTC (permalink / raw)
To: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 556 bytes --]
On Sunday, June 21, 2015 14:42:49 Sven Eckelmann wrote:
> Invalid speed settings by the user are currently acknowledged as correct
> but not stored. Instead the return of the store operation of the file
> "gw_bandwidth" should indicate that the given value is not acceptable.
>
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
> v2:
> * rebased on current master
> * add missing header linux/errno.h
>
> net/batman-adv/gateway_common.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Applied in revision aa203f7.
Thanks,
Marek
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH v2 4/4] batman-adv: Fix gw_bandwidth calculation on 32 bit systems
2015-06-21 12:42 ` [B.A.T.M.A.N.] [PATCH v2 4/4] batman-adv: Fix gw_bandwidth calculation on 32 bit systems Sven Eckelmann
@ 2015-06-29 9:47 ` Antonio Quartulli
2015-06-29 16:16 ` Sven Eckelmann
2015-07-10 8:04 ` Marek Lindner
1 sibling, 1 reply; 11+ messages in thread
From: Antonio Quartulli @ 2015-06-29 9:47 UTC (permalink / raw)
To: Sven Eckelmann, Marek Lindner
Cc: The list for a Better Approach To Mobile Ad-hoc Networking
[-- Attachment #1: Type: text/plain, Size: 856 bytes --]
On 21/06/15 14:42, Sven Eckelmann wrote:
> The TVLV for the gw_bandwidth stores everything as u32. But the
> gw_bandwidth reads the signed long which limits the maximum value to
> (2 ** 31 - 1) on systems with 4 byte long. Also the input value is always
> converted from either Mibit/s or Kibit/s to 100Kibit/s. This reduces the
> values even further when the user sets it via the default unit Kibit/s. It
> may even cause an integer overflow and end up with a value the user never
> intended.
>
> Instead read the values as u64, check for possible overflows, do the unit
> adjustments and then reduce the size to u32.
>
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
> v2:
> * rebased on current master
shouldn't this patch be for maint as it is also fixing a potential
overflow issue?
--
Antonio Quartulli
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH v2 4/4] batman-adv: Fix gw_bandwidth calculation on 32 bit systems
2015-06-29 9:47 ` Antonio Quartulli
@ 2015-06-29 16:16 ` Sven Eckelmann
2015-06-29 16:36 ` Antonio Quartulli
0 siblings, 1 reply; 11+ messages in thread
From: Sven Eckelmann @ 2015-06-29 16:16 UTC (permalink / raw)
To: Antonio Quartulli
Cc: The list for a Better Approach To Mobile Ad-hoc Networking,
Marek Lindner
[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]
On Monday 29 June 2015 11:47:00 Antonio Quartulli wrote:
>
> On 21/06/15 14:42, Sven Eckelmann wrote:
> > The TVLV for the gw_bandwidth stores everything as u32. But the
> > gw_bandwidth reads the signed long which limits the maximum value to
> > (2 ** 31 - 1) on systems with 4 byte long. Also the input value is always
> > converted from either Mibit/s or Kibit/s to 100Kibit/s. This reduces the
> > values even further when the user sets it via the default unit Kibit/s. It
> > may even cause an integer overflow and end up with a value the user never
> > intended.
> >
> > Instead read the values as u64, check for possible overflows, do the unit
> > adjustments and then reduce the size to u32.
> >
> > Signed-off-by: Sven Eckelmann <sven@narfation.org>
> > ---
> > v2:
> > * rebased on current master
>
> shouldn't this patch be for maint as it is also fixing a potential
> overflow issue?
Only the configuration for users is not correctly stored. For example
4294967296Mbit would result in 0.1 Mbit instead of showing a failure. This is
hopefully not the biggest problem.
Just tell me if you want it resubmitted against maint (Linux 4.1.x).
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH v2 4/4] batman-adv: Fix gw_bandwidth calculation on 32 bit systems
2015-06-29 16:16 ` Sven Eckelmann
@ 2015-06-29 16:36 ` Antonio Quartulli
0 siblings, 0 replies; 11+ messages in thread
From: Antonio Quartulli @ 2015-06-29 16:36 UTC (permalink / raw)
To: Sven Eckelmann
Cc: The list for a Better Approach To Mobile Ad-hoc Networking,
Marek Lindner
[-- Attachment #1: Type: text/plain, Size: 527 bytes --]
On 29/06/15 18:16, Sven Eckelmann wrote:
> Only the configuration for users is not correctly stored. For example
> 4294967296Mbit would result in 0.1 Mbit instead of showing a failure. This is
> hopefully not the biggest problem.
Thanks for the explanation. I don't think we should consider this as a
fix then - it looks rather uncritical and rare.
>
> Just tell me if you want it resubmitted against maint (Linux 4.1.x).
No, thanks. This is not required at this point.
Cheers,
--
Antonio Quartulli
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH v2 2/4] batman-adv: Backport kstrtou64 for Linux < 2.6.39
2015-06-21 12:42 ` [B.A.T.M.A.N.] [PATCH v2 2/4] batman-adv: Backport kstrtou64 for Linux < 2.6.39 Sven Eckelmann
@ 2015-07-10 8:01 ` Marek Lindner
0 siblings, 0 replies; 11+ messages in thread
From: Marek Lindner @ 2015-07-10 8:01 UTC (permalink / raw)
To: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 287 bytes --]
On Sunday, June 21, 2015 14:42:50 Sven Eckelmann wrote:
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
> v2:
> * rebased on current master
>
> compat-include/linux/kernel.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
Applied in revision 194b581.
Thanks,
Marek
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH v2 3/4] batman-adv: Define (u|s)(8|16|32|64) limits for Linux < 3.14
2015-06-21 12:42 ` [B.A.T.M.A.N.] [PATCH v2 3/4] batman-adv: Define (u|s)(8|16|32|64) limits for Linux < 3.14 Sven Eckelmann
@ 2015-07-10 8:02 ` Marek Lindner
0 siblings, 0 replies; 11+ messages in thread
From: Marek Lindner @ 2015-07-10 8:02 UTC (permalink / raw)
To: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 292 bytes --]
On Sunday, June 21, 2015 14:42:51 Sven Eckelmann wrote:
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
> v2:
> * rebased on current master
>
> compat-include/linux/kernel.h | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
Applied in revision 557adc4.
Thanks,
Marek
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH v2 4/4] batman-adv: Fix gw_bandwidth calculation on 32 bit systems
2015-06-21 12:42 ` [B.A.T.M.A.N.] [PATCH v2 4/4] batman-adv: Fix gw_bandwidth calculation on 32 bit systems Sven Eckelmann
2015-06-29 9:47 ` Antonio Quartulli
@ 2015-07-10 8:04 ` Marek Lindner
1 sibling, 0 replies; 11+ messages in thread
From: Marek Lindner @ 2015-07-10 8:04 UTC (permalink / raw)
To: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 917 bytes --]
On Sunday, June 21, 2015 14:42:52 Sven Eckelmann wrote:
> The TVLV for the gw_bandwidth stores everything as u32. But the
> gw_bandwidth reads the signed long which limits the maximum value to
> (2 ** 31 - 1) on systems with 4 byte long. Also the input value is always
> converted from either Mibit/s or Kibit/s to 100Kibit/s. This reduces the
> values even further when the user sets it via the default unit Kibit/s. It
> may even cause an integer overflow and end up with a value the user never
> intended.
>
> Instead read the values as u64, check for possible overflows, do the unit
> adjustments and then reduce the size to u32.
>
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
> v2:
> * rebased on current master
>
> net/batman-adv/gateway_common.c | 49
> +++++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+),
> 7 deletions(-)
Applied in revision ca6b86f.
Thanks,
Marek
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-07-10 8:04 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-21 12:42 [B.A.T.M.A.N.] [PATCH v2 1/4] batman-adv: Return EINVAL on invalid gw_bandwidth change Sven Eckelmann
2015-06-21 12:42 ` [B.A.T.M.A.N.] [PATCH v2 2/4] batman-adv: Backport kstrtou64 for Linux < 2.6.39 Sven Eckelmann
2015-07-10 8:01 ` Marek Lindner
2015-06-21 12:42 ` [B.A.T.M.A.N.] [PATCH v2 3/4] batman-adv: Define (u|s)(8|16|32|64) limits for Linux < 3.14 Sven Eckelmann
2015-07-10 8:02 ` Marek Lindner
2015-06-21 12:42 ` [B.A.T.M.A.N.] [PATCH v2 4/4] batman-adv: Fix gw_bandwidth calculation on 32 bit systems Sven Eckelmann
2015-06-29 9:47 ` Antonio Quartulli
2015-06-29 16:16 ` Sven Eckelmann
2015-06-29 16:36 ` Antonio Quartulli
2015-07-10 8:04 ` Marek Lindner
2015-06-28 14:40 ` [B.A.T.M.A.N.] [PATCH v2 1/4] batman-adv: Return EINVAL on invalid gw_bandwidth change Marek Lindner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).