pull request: batman-adv 2011-01-30

pull request: batman-adv 2011-01-30

[Sven Eckelmann]

Hi,

I would like to propose following patches for net-2.6.git/2.6.38. These are
important fixes for kernel oopses/memory leaks. I already send this exact same
request in 201101301036.57928.sven@narfation.org were you requested a new patch
series posting.

I cannot reduce the number of fixes any more because it makes no sense to fix
one Oops and leave the other one open. I've already removed the fixes for the
protocol implementation errors from the first patchset.

The following changes since commit aa0adb1a85e159cf57f0e11282bc6c9e3606a5f3:

  batman-adv: Use "__attribute__" shortcut macros (2011-01-16 03:25:19 +0100)

are available in the git repository at:
  git://git.open-mesh.org/ecsv/linux-merge.git batman-adv/merge-oopsonly

Linus Lüssing (1):
      batman-adv: Fix kernel panic when fetching vis data on a vis server

Sven Eckelmann (3):
      batman-adv: Remove vis info on hashing errors
      batman-adv: Remove vis info element in free_info
      batman-adv: Make vis info stack traversal threadsafe

 net/batman-adv/vis.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

 Thanks,
 	Sven

[PATCH 1/4] batman-adv: Fix kernel panic when fetching vis data on a vis server

[Sven Eckelmann]

From: Linus Lüssing <linus.luessing@web.de>

The hash_iterate removal introduced a bug leading to a kernel panic when
fetching the vis data on a vis server. That commit forgot to rename one
variable name, which this commit fixes now.

Reported-by: Russell Senior <russell@personaltelco.net>
Signed-off-by: Linus Lüssing <linus.luessing@web.de>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/vis.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/vis.c b/net/batman-adv/vis.c
index cd4c423..f69a374 100644
--- a/net/batman-adv/vis.c
+++ b/net/batman-adv/vis.c
@@ -268,10 +268,10 @@ int vis_seq_print_text(struct seq_file *seq, void *offset)
 				buff_pos += sprintf(buff + buff_pos, "%pM,",
 						entry->addr);
 
-				for (i = 0; i < packet->entries; i++)
+				for (j = 0; j < packet->entries; j++)
 					buff_pos += vis_data_read_entry(
 							buff + buff_pos,
-							&entries[i],
+							&entries[j],
 							entry->addr,
 							entry->primary);
 
-- 
1.7.2.3

[PATCH 2/4] batman-adv: Remove vis info on hashing errors

[Sven Eckelmann]

A newly created vis info object must be removed when it couldn't be
added to the hash. The old_info which has to be replaced was already
removed and isn't related to the hash anymore.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/vis.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/batman-adv/vis.c b/net/batman-adv/vis.c
index f69a374..0be55be 100644
--- a/net/batman-adv/vis.c
+++ b/net/batman-adv/vis.c
@@ -444,7 +444,7 @@ static struct vis_info *add_packet(struct bat_priv *bat_priv,
 			      info);
 	if (hash_added < 0) {
 		/* did not work (for some reason) */
-		kref_put(&old_info->refcount, free_info);
+		kref_put(&info->refcount, free_info);
 		info = NULL;
 	}
 
-- 
1.7.2.3

[PATCH 3/4] batman-adv: Remove vis info element in free_info

[Sven Eckelmann]

The free_info function will be called when no reference to the info
object exists anymore. It must be ensured that the allocated memory
gets freed and not only the elements which are managed by the info
object.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/vis.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/batman-adv/vis.c b/net/batman-adv/vis.c
index 0be55be..988296c 100644
--- a/net/batman-adv/vis.c
+++ b/net/batman-adv/vis.c
@@ -64,6 +64,7 @@ static void free_info(struct kref *ref)
 
 	spin_unlock_bh(&bat_priv->vis_list_lock);
 	kfree_skb(info->skb_packet);
+	kfree(info);
 }
 
 /* Compare two vis packets, used by the hashing algorithm */
-- 
1.7.2.3

[PATCH 4/4] batman-adv: Make vis info stack traversal threadsafe

[Sven Eckelmann]

The batman-adv vis server has to a stack which stores all information
about packets which should be send later. This stack is protected
with a spinlock that is used to prevent concurrent write access to it.

The send_vis_packets function has to take all elements from the stack
and send them to other hosts over the primary interface. The send will
be initiated without the lock which protects the stack.

The implementation using list_for_each_entry_safe has the problem that
it stores the next element as "safe ptr" to allow the deletion of the
current element in the list. The list may be modified during the
unlock/lock pair in the loop body which may make the safe pointer
not pointing to correct next element.

It is safer to remove and use the first element from the stack until no
elements are available. This does not need reduntant information which
would have to be validated each time the lock was removed.

Reported-by: Russell Senior <russell@personaltelco.net>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/vis.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/batman-adv/vis.c b/net/batman-adv/vis.c
index 988296c..de1022c 100644
--- a/net/batman-adv/vis.c
+++ b/net/batman-adv/vis.c
@@ -816,7 +816,7 @@ static void send_vis_packets(struct work_struct *work)
 		container_of(work, struct delayed_work, work);
 	struct bat_priv *bat_priv =
 		container_of(delayed_work, struct bat_priv, vis_work);
-	struct vis_info *info, *temp;
+	struct vis_info *info;
 
 	spin_lock_bh(&bat_priv->vis_hash_lock);
 	purge_vis_packets(bat_priv);
@@ -826,8 +826,9 @@ static void send_vis_packets(struct work_struct *work)
 		send_list_add(bat_priv, bat_priv->my_vis_info);
 	}
 
-	list_for_each_entry_safe(info, temp, &bat_priv->vis_send_list,
-				 send_list) {
+	while (!list_empty(&bat_priv->vis_send_list)) {
+		info = list_first_entry(&bat_priv->vis_send_list,
+					typeof(*info), send_list);
 
 		kref_get(&info->refcount);
 		spin_unlock_bh(&bat_priv->vis_hash_lock);
-- 
1.7.2.3

Re: pull request: batman-adv 2011-01-30

[David Miller]

From: Sven Eckelmann <sven@narfation.org>
Date: Sun, 30 Jan 2011 13:11:40 +0100

> Hi,
> 
> I would like to propose following patches for net-2.6.git/2.6.38. These are
> important fixes for kernel oopses/memory leaks. I already send this exact same
> request in 201101301036.57928.sven@narfation.org were you requested a new patch
> series posting.
> 
> I cannot reduce the number of fixes any more because it makes no sense to fix
> one Oops and leave the other one open. I've already removed the fixes for the
> protocol implementation errors from the first patchset.
> 
> The following changes since commit aa0adb1a85e159cf57f0e11282bc6c9e3606a5f3:
> 
>   batman-adv: Use "__attribute__" shortcut macros (2011-01-16 03:25:19 +0100)
> 
> are available in the git repository at:
>   git://git.open-mesh.org/ecsv/linux-merge.git batman-adv/merge-oopsonly

Pulled, thanks a lot.

Re: pull request: batman-adv 2011-01-29

[Sven Eckelmann]

[-- Attachment #1: Type: Text/Plain, Size: 1167 bytes --]

On Sunday 30 January 2011 04:08:15 David Miller wrote:
> I'm not taking 13 patches in one big bunch at this point post-merge-window,
> now matter what they are.

Thats why I tried to provide the 6 bug fixes only version without polishing - 
I could also try to reduce it to a 4 critical version (oops/memleak patches 
only without protocol implementation failures).

Or is the problem that I tried to get them in as pull request?

thanks,
	Sven


Here are the oops/memleak patches only:

The following changes since commit aa0adb1a85e159cf57f0e11282bc6c9e3606a5f3:

  batman-adv: Use "__attribute__" shortcut macros (2011-01-16 03:25:19 +0100)

are available in the git repository at:
  git://git.open-mesh.org/ecsv/linux-merge.git batman-adv/merge-oopsonly

Linus Lüssing (1):
      batman-adv: Fix kernel panic when fetching vis data on a vis server

Sven Eckelmann (3):
      batman-adv: Remove vis info on hashing errors
      batman-adv: Remove vis info element in free_info
      batman-adv: Make vis info stack traversal threadsafe

 net/batman-adv/vis.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

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

[PATCH 2/4] batman-adv: Remove vis info on hashing errors

[Sven Eckelmann]

A newly created vis info object must be removed when it couldn't be
added to the hash. The old_info which has to be replaced was already
removed and isn't related to the hash anymore.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/vis.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/batman-adv/vis.c b/net/batman-adv/vis.c
index f69a374..0be55be 100644
--- a/net/batman-adv/vis.c
+++ b/net/batman-adv/vis.c
@@ -444,7 +444,7 @@ static struct vis_info *add_packet(struct bat_priv *bat_priv,
 			      info);
 	if (hash_added < 0) {
 		/* did not work (for some reason) */
-		kref_put(&old_info->refcount, free_info);
+		kref_put(&info->refcount, free_info);
 		info = NULL;
 	}
 
-- 
1.7.2.3