nl802154: fix rtnl_unlock() being missing

From: Jean Sacren <[email protected]>

In nl802154_prepare_wpan_dev_dump(), rtnl_unlock() was missing if it returns 0. If we insert rtnl_unlock() mechanically, we will have unbearable code duplication. Fix this bug by unifying exit paths.

Following the new exit path, clean up now the obsolete goto statement.

---
 net/ieee802154/nl802154.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

index 1e9e86508441..0074f13ea4ca 100644
@@ -253,7 +253,7 @@ nl802154_prepare_wpan_dev_dump(struct sk_buff *skb,
 			       struct cfg802154_registered_device **rdev,
 			       struct wpan_dev **wpan_dev)
 {
-	int err;
+	int err = 0;
 
 	rtnl_lock();
 

@@ -293,13 +293,10 @@ nl802154_prepare_wpan_dev_dump(struct sk_buff *skb,
 			}
 		}
 
-		if (!*wpan_dev) {
+		if (!*wpan_dev)
 			err = -ENODEV;
-			goto out_unlock;
-		}
 	}
 
-	return 0;
  out_unlock:
 	rtnl_unlock();
 	return err;

Hi Jean,

There is no rtnl_unlock missing, the rtnl_unlock call will be done by function "nl802154_finish_wpan_dev_dump".

The rtnl lock need to be held at the full netlink _dump_ callback, e.g.:

1. call nl802154_prepare_wpan_dev_dump

2. doing netlink things

3. nl802154_finish_wpan_dev_dump

The 3. point is important and we have a bug if we don't call "nl802154_finish_wpan_dev_dump" at least. Except "nl802154_prepare_wpan_dev_dump" returns an error.

This mechanism was copy&pasted(with minor changes) from wireless which we use as reference design. If you like to change this behaviour, then you need to first change it in wireless, then here.

The identical function can be found at [0].

btw:
Doing some shared code between wireless/802.15.4 would be great but at first we try to reach some stable subsystem implementation.

- Alex

[0] http://lxr.free-electrons.com/source/net/wireless/nl80211.c#L481

Actually we have something like that for the "doit" callback. In this case netlink offers a "pre" and "post" callback for doing things before and after the netlink call, see [0].

Maybe another idea is to have a "pre_dumpit" and "post_dumpit", but then we need to care about dump callbacks which doesn't use "nl802154_prepare_wpan_dev_dump" - these callbacks exist. I have not exactly look at it and don't know how possible it is. It would be a nice cleanup. Maybe this can be workarounded by some FLAG to do WDEV_PEPARE or not. I don't know, just an idea. ;-)

But then do it at first in wireless subsystem. I also don't know if the wireless community likes this idea or not.

The "pre" "post" ensures that we always handles the rtnl locks correctly.

- Alex

[0]http://lxr.free-electrons.com/source/net/wireless/nl80211.c#L47

From: Alexander Aring <[email protected]>
Date: Sat, 03 Oct 2015 08:44:14 +0200

I'm so sorry. Thank you for teaching me again.