[B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: Fix segfault when writing to throughput

public inbox for b.a.t.m.a.n@lists.open-mesh.org
 help / color / mirror / Atom feed

* [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: Fix segfault when writing to throughput_override
@ 2018-08-31 14:46 Sven Eckelmann
  2018-08-31 14:46 ` [B.A.T.M.A.N.] [PATCH maint 2/2] batman-adv: Fix segfault when writing to sysfs elp_interval Sven Eckelmann
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Sven Eckelmann @ 2018-08-31 14:46 UTC (permalink / raw)
  To: b.a.t.m.a.n

The per hardif sysfs file "batman_adv/throughput_override" prints the
resulting change as info text when the users writes to this file. It uses
the helper function batadv_info to add it at the same time to the kernel
ring buffer and to the batman-adv debug log (when CONFIG_BATMAN_ADV_DEBUG
is enabled).

The function batadv_info requires as first parameter the batman-adv softif
net_device. This parameter is then used to find the private buffer which
contains the debug log for this batman-adv interface. But
batadv_store_throughput_override used as first argument the slave
net_device. This slave device doesn't have the batadv_priv private data
which is access by batadv_info.

Writing to this file with CONFIG_BATMAN_ADV_DEBUG enabled can either lead
to a segfault or to memory corruption.

Fixes: c513176e4b7a ("batman-adv: add throughput override attribute to hard_ifaces")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/sysfs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
index f2eef43b..3a76e897 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -1090,8 +1090,9 @@ static ssize_t batadv_store_throughput_override(struct kobject *kobj,
 	if (old_tp_override == tp_override)
 		goto out;

-	batadv_info(net_dev, "%s: Changing from: %u.%u MBit to: %u.%u MBit\n",
-		    "throughput_override",
+	batadv_info(hard_iface->soft_iface,
+		    "%s: %s: Changing from: %u.%u MBit to: %u.%u MBit\n",
+		    "throughput_override", net_dev->name,
 		    old_tp_override / 10, old_tp_override % 10,
 		    tp_override / 10, tp_override % 10);

-- 
2.18.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [B.A.T.M.A.N.] [PATCH maint 2/2] batman-adv: Fix segfault when writing to sysfs elp_interval
  2018-08-31 14:46 [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: Fix segfault when writing to throughput_override Sven Eckelmann
@ 2018-08-31 14:46 ` Sven Eckelmann
  2018-08-31 14:56 ` [B.A.T.M.A.N.] [PATCH maint v2 " Sven Eckelmann
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Sven Eckelmann @ 2018-08-31 14:46 UTC (permalink / raw)
  To: b.a.t.m.a.n

The per hardif sysfs file "batman_adv/elp_interval" is using the generic
functions to store/show uint values. The helper __batadv_store_uint_attr
requires the softif net_device as parameter to print the resulting change
as info text when the users writes to this file. It uses the helper
function batadv_info to add it at the same time to the kernel ring buffer
and to the batman-adv debug log (when CONFIG_BATMAN_ADV_DEBUG is enabled).

The function batadv_info requires as first parameter the batman-adv softif
net_device. This parameter is then used to find the private buffer which
contains the debug log for this batman-adv interface. But
batadv_store_throughput_override used as first argument the slave
net_device. This slave device doesn't have the batadv_priv private data
which is access by batadv_info.

Writing to this file with CONFIG_BATMAN_ADV_DEBUG enabled can either lead
to a segfault or to memory corruption.

Fixes: ec46535b8275 ("batman-adv: Add hard_iface specific sysfs wrapper macros for UINT")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/sysfs.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
index 3a76e897..b2e7bb51 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -188,7 +188,8 @@ ssize_t batadv_store_##_name(struct kobject *kobj,			\
 									\
 	return __batadv_store_uint_attr(buff, count, _min, _max,	\
 					_post_func, attr,		\
-					&bat_priv->_var, net_dev);	\
+					&bat_priv->_var, net_dev,	\
+					NULL);	\
 }
 
 #define BATADV_ATTR_SIF_SHOW_UINT(_name, _var)				\
@@ -262,7 +263,9 @@ ssize_t batadv_store_##_name(struct kobject *kobj,			\
 									\
 	length = __batadv_store_uint_attr(buff, count, _min, _max,	\
 					  _post_func, attr,		\
-					  &hard_iface->_var, net_dev);	\
+					  &hard_iface->_var,		\
+					  hard_iface->soft_iface,	\
+					  net_dev);			\
 									\
 	batadv_hardif_put(hard_iface);				\
 	return length;							\
@@ -356,10 +359,12 @@ __batadv_store_bool_attr(char *buff, size_t count,
 
 static int batadv_store_uint_attr(const char *buff, size_t count,
 				  struct net_device *net_dev,
+				  struct net_device *slave_dev,
 				  const char *attr_name,
 				  unsigned int min, unsigned int max,
 				  atomic_t *attr)
 {
+	char ifname[IFNAMSIZ + 3] = "\0";
 	unsigned long uint_val;
 	int ret;
 
@@ -385,8 +390,11 @@ static int batadv_store_uint_attr(const char *buff, size_t count,
 	if (atomic_read(attr) == uint_val)
 		return count;
 
-	batadv_info(net_dev, "%s: Changing from: %i to: %lu\n",
-		    attr_name, atomic_read(attr), uint_val);
+	if (slave_dev)
+		snprintf(ifname, sizeof(ifname), "%s: ", slave_dev->name);
+
+	batadv_info(net_dev, "%s: %sChanging from: %i to: %lu\n",
+		    attr_name, ifname, atomic_read(attr), uint_val);
 
 	atomic_set(attr, uint_val);
 	return count;
@@ -397,12 +405,13 @@ static ssize_t __batadv_store_uint_attr(const char *buff, size_t count,
 					void (*post_func)(struct net_device *),
 					const struct attribute *attr,
 					atomic_t *attr_store,
-					struct net_device *net_dev)
+					struct net_device *net_dev,
+					struct net_device *slave_dev)
 {
 	int ret;
 
-	ret = batadv_store_uint_attr(buff, count, net_dev, attr->name, min, max,
-				     attr_store);
+	ret = batadv_store_uint_attr(buff, count, net_dev, slave_dev,
+				     attr->name, min, max, attr_store);
 	if (post_func && ret)
 		post_func(net_dev);
 
@@ -571,7 +580,7 @@ static ssize_t batadv_store_gw_sel_class(struct kobject *kobj,
 	return __batadv_store_uint_attr(buff, count, 1, BATADV_TQ_MAX_VALUE,
 					batadv_post_gw_reselect, attr,
 					&bat_priv->gw.sel_class,
-					bat_priv->soft_iface);
+					bat_priv->soft_iface, NULL);
 }
 
 static ssize_t batadv_show_gw_bwidth(struct kobject *kobj,
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [B.A.T.M.A.N.] [PATCH maint v2 2/2] batman-adv: Fix segfault when writing to sysfs elp_interval
  2018-08-31 14:46 [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: Fix segfault when writing to throughput_override Sven Eckelmann
  2018-08-31 14:46 ` [B.A.T.M.A.N.] [PATCH maint 2/2] batman-adv: Fix segfault when writing to sysfs elp_interval Sven Eckelmann
@ 2018-08-31 14:56 ` Sven Eckelmann
  2018-09-05 11:09   ` Marek Lindner
  2018-09-05 10:55 ` [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: Fix segfault when writing to throughput_override Marek Lindner
  2018-09-05 16:07 ` Sven Eckelmann
  3 siblings, 1 reply; 6+ messages in thread
From: Sven Eckelmann @ 2018-08-31 14:56 UTC (permalink / raw)
  To: b.a.t.m.a.n

The per hardif sysfs file "batman_adv/elp_interval" is using the generic
functions to store/show uint values. The helper __batadv_store_uint_attr
requires the softif net_device as parameter to print the resulting change
as info text when the users writes to this file. It uses the helper
function batadv_info to add it at the same time to the kernel ring buffer
and to the batman-adv debug log (when CONFIG_BATMAN_ADV_DEBUG is enabled).

The function batadv_info requires as first parameter the batman-adv softif
net_device. This parameter is then used to find the private buffer which
contains the debug log for this batman-adv interface. But
batadv_store_throughput_override used as first argument the slave
net_device. This slave device doesn't have the batadv_priv private data
which is access by batadv_info.

Writing to this file with CONFIG_BATMAN_ADV_DEBUG enabled can either lead
to a segfault or to memory corruption.

Fixes: ec46535b8275 ("batman-adv: Add hard_iface specific sysfs wrapper macros for UINT")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
v2:

* changed how ifname gets initialized (not sure why I've added \0)

 net/batman-adv/sysfs.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
index 3a76e897..09427fc6 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -188,7 +188,8 @@ ssize_t batadv_store_##_name(struct kobject *kobj,			\
 									\
 	return __batadv_store_uint_attr(buff, count, _min, _max,	\
 					_post_func, attr,		\
-					&bat_priv->_var, net_dev);	\
+					&bat_priv->_var, net_dev,	\
+					NULL);	\
 }
 
 #define BATADV_ATTR_SIF_SHOW_UINT(_name, _var)				\
@@ -262,7 +263,9 @@ ssize_t batadv_store_##_name(struct kobject *kobj,			\
 									\
 	length = __batadv_store_uint_attr(buff, count, _min, _max,	\
 					  _post_func, attr,		\
-					  &hard_iface->_var, net_dev);	\
+					  &hard_iface->_var,		\
+					  hard_iface->soft_iface,	\
+					  net_dev);			\
 									\
 	batadv_hardif_put(hard_iface);				\
 	return length;							\
@@ -356,10 +359,12 @@ __batadv_store_bool_attr(char *buff, size_t count,
 
 static int batadv_store_uint_attr(const char *buff, size_t count,
 				  struct net_device *net_dev,
+				  struct net_device *slave_dev,
 				  const char *attr_name,
 				  unsigned int min, unsigned int max,
 				  atomic_t *attr)
 {
+	char ifname[IFNAMSIZ + 3] = "";
 	unsigned long uint_val;
 	int ret;
 
@@ -385,8 +390,11 @@ static int batadv_store_uint_attr(const char *buff, size_t count,
 	if (atomic_read(attr) == uint_val)
 		return count;
 
-	batadv_info(net_dev, "%s: Changing from: %i to: %lu\n",
-		    attr_name, atomic_read(attr), uint_val);
+	if (slave_dev)
+		snprintf(ifname, sizeof(ifname), "%s: ", slave_dev->name);
+
+	batadv_info(net_dev, "%s: %sChanging from: %i to: %lu\n",
+		    attr_name, ifname, atomic_read(attr), uint_val);
 
 	atomic_set(attr, uint_val);
 	return count;
@@ -397,12 +405,13 @@ static ssize_t __batadv_store_uint_attr(const char *buff, size_t count,
 					void (*post_func)(struct net_device *),
 					const struct attribute *attr,
 					atomic_t *attr_store,
-					struct net_device *net_dev)
+					struct net_device *net_dev,
+					struct net_device *slave_dev)
 {
 	int ret;
 
-	ret = batadv_store_uint_attr(buff, count, net_dev, attr->name, min, max,
-				     attr_store);
+	ret = batadv_store_uint_attr(buff, count, net_dev, slave_dev,
+				     attr->name, min, max, attr_store);
 	if (post_func && ret)
 		post_func(net_dev);
 
@@ -571,7 +580,7 @@ static ssize_t batadv_store_gw_sel_class(struct kobject *kobj,
 	return __batadv_store_uint_attr(buff, count, 1, BATADV_TQ_MAX_VALUE,
 					batadv_post_gw_reselect, attr,
 					&bat_priv->gw.sel_class,
-					bat_priv->soft_iface);
+					bat_priv->soft_iface, NULL);
 }
 
 static ssize_t batadv_show_gw_bwidth(struct kobject *kobj,
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH maint v2 2/2] batman-adv: Fix segfault when writing to sysfs elp_interval
  2018-08-31 14:56 ` [B.A.T.M.A.N.] [PATCH maint v2 " Sven Eckelmann
@ 2018-09-05 11:09   ` Marek Lindner
  0 siblings, 0 replies; 6+ messages in thread
From: Marek Lindner @ 2018-09-05 11:09 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

[-- Attachment #1: Type: text/plain, Size: 1238 bytes --]

On Friday, 31 August 2018 22:56:29 HKT Sven Eckelmann wrote:
> The per hardif sysfs file "batman_adv/elp_interval" is using the generic
> functions to store/show uint values. The helper __batadv_store_uint_attr
> requires the softif net_device as parameter to print the resulting change
> as info text when the users writes to this file. It uses the helper
> function batadv_info to add it at the same time to the kernel ring buffer
> and to the batman-adv debug log (when CONFIG_BATMAN_ADV_DEBUG is enabled).
> 
> The function batadv_info requires as first parameter the batman-adv softif
> net_device. This parameter is then used to find the private buffer which
> contains the debug log for this batman-adv interface. But
> batadv_store_throughput_override used as first argument the slave
> net_device. This slave device doesn't have the batadv_priv private data
> which is access by batadv_info.
> 
> Writing to this file with CONFIG_BATMAN_ADV_DEBUG enabled can either lead
> to a segfault or to memory corruption.
> 
> Fixes: ec46535b8275 ("batman-adv: Add hard_iface specific sysfs wrapper
> macros for UINT") Signed-off-by: Sven Eckelmann <sven@narfation.org>

Acked-by: Marek Lindner <mareklindner@neomailbox.ch>

Cheers,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: Fix segfault when writing to throughput_override
  2018-08-31 14:46 [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: Fix segfault when writing to throughput_override Sven Eckelmann
  2018-08-31 14:46 ` [B.A.T.M.A.N.] [PATCH maint 2/2] batman-adv: Fix segfault when writing to sysfs elp_interval Sven Eckelmann
  2018-08-31 14:56 ` [B.A.T.M.A.N.] [PATCH maint v2 " Sven Eckelmann
@ 2018-09-05 10:55 ` Marek Lindner
  2018-09-05 16:07 ` Sven Eckelmann
  3 siblings, 0 replies; 6+ messages in thread
From: Marek Lindner @ 2018-09-05 10:55 UTC (permalink / raw)
  To: b.a.t.m.a.n

[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]

On Friday, 31 August 2018 22:46:47 HKT Sven Eckelmann wrote:
> The per hardif sysfs file "batman_adv/throughput_override" prints the
> resulting change as info text when the users writes to this file. It uses
> the helper function batadv_info to add it at the same time to the kernel
> ring buffer and to the batman-adv debug log (when CONFIG_BATMAN_ADV_DEBUG
> is enabled).
> 
> The function batadv_info requires as first parameter the batman-adv softif
> net_device. This parameter is then used to find the private buffer which
> contains the debug log for this batman-adv interface. But
> batadv_store_throughput_override used as first argument the slave
> net_device. This slave device doesn't have the batadv_priv private data
> which is access by batadv_info.
> 
> Writing to this file with CONFIG_BATMAN_ADV_DEBUG enabled can either lead
> to a segfault or to memory corruption.
> 
> Fixes: c513176e4b7a ("batman-adv: add throughput override attribute to
> hard_ifaces") Signed-off-by: Sven Eckelmann <sven@narfation.org>


Acked-by: Marek Lindner <mareklindner@neomailbox.ch>

Cheers,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: Fix segfault when writing to throughput_override
  2018-08-31 14:46 [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: Fix segfault when writing to throughput_override Sven Eckelmann
                   ` (2 preceding siblings ...)
  2018-09-05 10:55 ` [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: Fix segfault when writing to throughput_override Marek Lindner
@ 2018-09-05 16:07 ` Sven Eckelmann
  3 siblings, 0 replies; 6+ messages in thread
From: Sven Eckelmann @ 2018-09-05 16:07 UTC (permalink / raw)
  To: b.a.t.m.a.n

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]

On Freitag, 31. August 2018 16:46:47 CEST Sven Eckelmann wrote:
> The per hardif sysfs file "batman_adv/throughput_override" prints the
> resulting change as info text when the users writes to this file. It uses
> the helper function batadv_info to add it at the same time to the kernel
> ring buffer and to the batman-adv debug log (when CONFIG_BATMAN_ADV_DEBUG
> is enabled).
> 
> The function batadv_info requires as first parameter the batman-adv softif
> net_device. This parameter is then used to find the private buffer which
> contains the debug log for this batman-adv interface. But
> batadv_store_throughput_override used as first argument the slave
> net_device. This slave device doesn't have the batadv_priv private data
> which is access by batadv_info.
> 
> Writing to this file with CONFIG_BATMAN_ADV_DEBUG enabled can either lead
> to a segfault or to memory corruption.
[...]

Added both patches as ddf99b78e255 [1] and 848be9859b01 [2]

Kind regards,
	Sven

[1] https://git.open-mesh.org/batman-adv.git/commit/ddf99b78e255530cbadc0f67656a549e19520280
[2] https://git.open-mesh.org/batman-adv.git/commit/848be9859b0109a6e428f92f21f2e660153b1c75

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-09-05 16:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-31 14:46 [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: Fix segfault when writing to throughput_override Sven Eckelmann
2018-08-31 14:46 ` [B.A.T.M.A.N.] [PATCH maint 2/2] batman-adv: Fix segfault when writing to sysfs elp_interval Sven Eckelmann
2018-08-31 14:56 ` [B.A.T.M.A.N.] [PATCH maint v2 " Sven Eckelmann
2018-09-05 11:09   ` Marek Lindner
2018-09-05 10:55 ` [B.A.T.M.A.N.] [PATCH maint 1/2] batman-adv: Fix segfault when writing to throughput_override Marek Lindner
2018-09-05 16:07 ` Sven Eckelmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox