[PATCH] decnet: handle a failure in neigh_parms_alloc

While enhancing the neighbour code to handle multiple network
namespaces I noticed that decnet is assuming neigh_parms_alloc
will allways succeed, which is clearly wrong.  So handle the
failure.

Signed-off-by: Eric W. Biederman <redacted>
---
 net/decnet/dn_dev.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/net/decnet/dn_dev.c b/net/decnet/dn_dev.c
index 324eb47..f6a2b2a 100644
--- a/net/decnet/dn_dev.c
+++ b/net/decnet/dn_dev.c

@@ -1149,6 +1149,11 @@ struct dn_dev *dn_dev_create(struct net_device *dev, int *err)
 	}
 
 	dn_db->neigh_parms = neigh_parms_alloc(dev, &dn_neigh_table);
+	if (!dn_db->neigh_parms) {
+		dev->dn_ptr = NULL;
+		kfree(dn_db);
+		return NULL;
+	}
 
 	dn_dev_sysctl_register(dev, &dn_db->parms);
 

-- 
1.4.4.1.g278f

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Wed, 24 Jan 2007 13:09:40 -0700

While enhancing the neighbour code to handle multiple network
namespaces I noticed that decnet is assuming neigh_parms_alloc
will allways succeed, which is clearly wrong.  So handle the
failure.

Signed-off-by: Eric W. Biederman <redacted>

I don't think this one is correct.

During failure cleanup you also have to invoke the neighbour parms
->down() operation if present, else you leave multicast entries on the
ethernet device for example.

From: David Miller <davem@davemloft.net>
Date: Wed, 24 Jan 2007 14:45:11 -0800 (PST)

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Wed, 24 Jan 2007 13:09:40 -0700

quoted

While enhancing the neighbour code to handle multiple network
namespaces I noticed that decnet is assuming neigh_parms_alloc
will allways succeed, which is clearly wrong.  So handle the
failure.

Signed-off-by: Eric W. Biederman <redacted>

I don't think this one is correct.

During failure cleanup you also have to invoke the neighbour parms
->down() operation if present, else you leave multicast entries on the
ethernet device for example.

BTW, it occurs to me that a good way to handle this might be to
try and do the neigh_parms_alloc() before running the ->up()
method.

While enhancing the neighbour code to handle multiple network
namespaces I noticed that decnet is assuming neigh_parms_alloc
will allways succeed, which is clearly wrong.  So handle the
failure.

Signed-off-by: Eric W. Biederman <redacted>
---
 net/decnet/dn_dev.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/decnet/dn_dev.c b/net/decnet/dn_dev.c
index 324eb47..913e25a 100644
--- a/net/decnet/dn_dev.c
+++ b/net/decnet/dn_dev.c

@@ -1140,16 +1140,23 @@ struct dn_dev *dn_dev_create(struct net_device *dev, int *err)
 	init_timer(&dn_db->timer);
 
 	dn_db->uptime = jiffies;
+
+	dn_db->neigh_parms = neigh_parms_alloc(dev, &dn_neigh_table);
+	if (!dn_db->neigh_parms) {
+		dev->dn_ptr = NULL;
+		kfree(dn_db);
+		return NULL;
+	}
+
 	if (dn_db->parms.up) {
 		if (dn_db->parms.up(dev) < 0) {
+			neigh_parms_release(&dn_neigh_table, dn_db->neigh_parms);
 			dev->dn_ptr = NULL;
 			kfree(dn_db);
 			return NULL;
 		}
 	}
 
-	dn_db->neigh_parms = neigh_parms_alloc(dev, &dn_neigh_table);
-
 	dn_dev_sysctl_register(dev, &dn_db->parms);
 
 	dn_dev_set_timer(dev);

-- 
1.4.4.1.g278f

Hi,

On Wed, Jan 24, 2007 at 09:55:45PM -0700, Eric W. Biederman wrote:

While enhancing the neighbour code to handle multiple network
namespaces I noticed that decnet is assuming neigh_parms_alloc
will allways succeed, which is clearly wrong.  So handle the
failure.

Signed-off-by: Eric W. Biederman <redacted>

Acked-by: Steven Whitehouse <redacted>

Also you should cc Patrick as he is now the maintainer,

Steve.

From: Steven Whitehouse <redacted>
Date: Thu, 25 Jan 2007 11:43:18 +0000

Hi,

On Wed, Jan 24, 2007 at 09:55:45PM -0700, Eric W. Biederman wrote:

quoted

While enhancing the neighbour code to handle multiple network
namespaces I noticed that decnet is assuming neigh_parms_alloc
will allways succeed, which is clearly wrong.  So handle the
failure.

Signed-off-by: Eric W. Biederman <redacted>

Acked-by: Steven Whitehouse <redacted>

Applied, thanks everyone.

Also you should cc Patrick as he is now the maintainer,

Yep, would be a good idea in the future.