[RFC] mac80211: Reorder debugfs calls during netdev deinit

[RFC] mac80211: Reorder debugfs calls during netdev deinit

[Jouni Malinen]

There was some discussion about the order in which debugfs entries are
removed in context of IEEE 802.11w implementation couple of months ago.
The problem that I was seeing ended up leaving the netdev:mon.wlan0
directory in debugfs behind when the monitor interface was removed. This
happened because mac80211 tried to remove the netdev directory before
making sure that all files were removed from the directory. This did not
show up before, but with IEEE 802.11w implementation, the
netdev:mon.wlan0 directory may actually contain a key symlink and that
prevented the directory from being removed.

It looks like someone has cleaned up the netdev uninit sequence (which
was quite a mess at the time I looked at it last and ended up just
delaying the fix till now..) and that seems to make it trivial to fix
the deinit order issue. The key part in fixing this is to make sure that
the possible key symlinks are removed before the netdev directory and
this is now easy to do by just changing the order of the function calls
in ieee80211_teardown_sdata(). I did not find any reason why they would
need to be in the other order, so this looks like a safe change. Did I
miss something there or would the following change be acceptable way of
fixing the issue? I did not notice any problems in my tests and this
allows the debugfs directories to be removed properly with my IEEE
802.11w patches applied.



ieee80211_free_keys() must be called before
ieee80211_debugfs_remove_netdev() in order to make sure that the
possible default_key symlink is removed before the netdev debugfs
directory is removed.

This fixes an issue where a monitor interface may be left behind when
being removed if there is a key symlink in it. This does not happen
with the current mac80211 code, but could happen in future after IEEE
802.11w (management frame protection) is added with its additional
default_mgmt_key symlink.

Signed-off-by: Jouni Malinen <jouni.malinen@atheros.com>


Index: wireless-testing/net/mac80211/iface.c
===================================================================
--- wireless-testing.orig/net/mac80211/iface.c
+++ wireless-testing/net/mac80211/iface.c
@@ -31,11 +31,11 @@ static void ieee80211_teardown_sdata(str
 	int flushed;
 	int i;
 
-	ieee80211_debugfs_remove_netdev(sdata);
-
 	/* free extra data */
 	ieee80211_free_keys(sdata);
 
+	ieee80211_debugfs_remove_netdev(sdata);
+
 	for (i = 0; i < IEEE80211_FRAGMENT_MAX; i++)
 		__skb_queue_purge(&sdata->fragments[i].skb_list);
 	sdata->fragment_next = 0;


-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [RFC] mac80211: Reorder debugfs calls during netdev deinit

[Johannes Berg]

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


> It looks like someone has cleaned up the netdev uninit sequence (which
> was quite a mess at the time I looked at it last and ended up just
> delaying the fix till now..) and that seems to make it trivial to fix
> the deinit order issue.

Yeah, that was me, I mostly rewrote the junk in iface.c.

> ieee80211_free_keys() must be called before
> ieee80211_debugfs_remove_netdev() in order to make sure that the
> possible default_key symlink is removed before the netdev debugfs
> directory is removed.
> 
> This fixes an issue where a monitor interface may be left behind when
> being removed if there is a key symlink in it. This does not happen
> with the current mac80211 code, but could happen in future after IEEE
> 802.11w (management frame protection) is added with its additional
> default_mgmt_key symlink.
> 
> Signed-off-by: Jouni Malinen <jouni.malinen@atheros.com>
> 
> 
> Index: wireless-testing/net/mac80211/iface.c
> ===================================================================
> --- wireless-testing.orig/net/mac80211/iface.c
> +++ wireless-testing/net/mac80211/iface.c
> @@ -31,11 +31,11 @@ static void ieee80211_teardown_sdata(str
>  	int flushed;
>  	int i;
>  
> -	ieee80211_debugfs_remove_netdev(sdata);
> -
>  	/* free extra data */
>  	ieee80211_free_keys(sdata);
>  
> +	ieee80211_debugfs_remove_netdev(sdata);
> +
>  	for (i = 0; i < IEEE80211_FRAGMENT_MAX; i++)
>  		__skb_queue_purge(&sdata->fragments[i].skb_list);
>  	sdata->fragment_next = 0;

Seems sane to me.

Not that I like the workaround of setting keys on the monitor, in fact,
I think it shouldn't be allowed ;)

johannes

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

Re: [RFC] mac80211: Reorder debugfs calls during netdev deinit

[Jouni Malinen]

On Thu, Aug 28, 2008 at 04:28:54PM +0200, Johannes Berg wrote:

> > It looks like someone has cleaned up the netdev uninit sequence (which
> > was quite a mess at the time I looked at it last and ended up just
> > delaying the fix till now..) and that seems to make it trivial to fix
> > the deinit order issue.
> 
> Yeah, that was me, I mostly rewrote the junk in iface.c.

Thanks! That saved lots of work.

> Seems sane to me.
> 
> Not that I like the workaround of setting keys on the monitor, in fact,
> I think it shouldn't be allowed ;)

Well, I don't like it either and it is next on my list for 802.11w
work.. I just needed to get mac80211 into a state where 802.11w works
and I do not need to reboot every time when I re-load mac80211 ;-).
Anyway, this reordering of the calls is correct even if the key was not
set for the monitor netdev; I just don't know whether there is any other
way to trigger this particular issue.

-- 
Jouni Malinen                                            PGP id EFC895FA