[PATCH] mac80211: allow key deletion for mesh interface

[PATCH] mac80211: allow key deletion for mesh interface

[Chun-Yeow Yeoh]

This is to support key deletion for mesh interface, especially
to be used for key which is not deleted even with the deletion
of peer mesh station.

Signed-off-by: Chun-Yeow Yeoh <yeohchunyeow@gmail.com>
---
 net/mac80211/cfg.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 64cf294..6ff3414 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -239,7 +239,10 @@ static int ieee80211_del_key(struct wiphy *wiphy, struct net_device *dev,
 	if (mac_addr) {
 		ret = -ENOENT;
 
-		sta = sta_info_get_bss(sdata, mac_addr);
+		if (ieee80211_vif_is_mesh(&sdata->vif))
+			sta = sta_info_get(sdata, mac_addr);
+		else
+			sta = sta_info_get_bss(sdata, mac_addr);
 		if (!sta)
 			goto out_unlock;
 
-- 
1.7.0.4

Re: [PATCH] mac80211: allow key deletion for mesh interface

[Johannes Berg]

On Tue, 2013-06-18 at 12:07 +0800, Chun-Yeow Yeoh wrote:
> This is to support key deletion for mesh interface, especially
> to be used for key which is not deleted even with the deletion
> of peer mesh station.

Can you explain which keys in mesh aren't deleted? It seems the
per-station keys would be deleted when the station is deleted, and
something like "GTK" would be deleted when leaving the mesh?

johannes

Re: [PATCH] mac80211: allow key deletion for mesh interface

[Johannes Berg]

On Tue, 2013-06-18 at 12:07 +0800, Chun-Yeow Yeoh wrote:

> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -239,7 +239,10 @@ static int ieee80211_del_key(struct wiphy *wiphy, struct net_device *dev,
>  	if (mac_addr) {
>  		ret = -ENOENT;
>  
> -		sta = sta_info_get_bss(sdata, mac_addr);
> +		if (ieee80211_vif_is_mesh(&sdata->vif))
> +			sta = sta_info_get(sdata, mac_addr);
> +		else
> +			sta = sta_info_get_bss(sdata, mac_addr);

I don't see that this actually changes anything. The mesh sdata will
have a NULL bss pointer, so the second condition in sta_info_get_bss()
can't be true.

Therefore, only the first condition can ever be considered, which is
exactly the same as sta_info_get(), no?

johannes

Re: [PATCH] mac80211: allow key deletion for mesh interface

[Yeoh Chun-Yeow]

> I don't see that this actually changes anything. The mesh sdata will
> have a NULL bss pointer, so the second condition in sta_info_get_bss()
> can't be true.
>
> Therefore, only the first condition can ever be considered, which is
> exactly the same as sta_info_get(), no?

Yes. My current patch does not make sense since I see no difference
calling sta_info_get or sta_info_get_bss in my case.

The same applies to ieee80211_add_key.

---
Chun-Yeow

Re: [PATCH] mac80211: allow key deletion for mesh interface

[Yeoh Chun-Yeow]

In the ieee80211_free_sta_keys, the key idx 4 is not deleted, since it
is limited by NUM_DEFAULT_KEYS.

----
Chun-Yeow

On Tue, Jun 18, 2013 at 3:56 PM, Yeoh Chun-Yeow <yeohchunyeow@gmail.com> wrote:
> In authsae, we have a GTK with key index 4 as follow:
> install_key(&nlcfg, peer, CIPHER_AES_CMAC, NL80211_KEYTYPE_GROUP, 4, peer_mgtk);
>
> We may need to delete this key while doing re-authentication for the
> same peer mesh STA due to accidental reboot. Otherwise, this GTK key
> is not deleted.
>
> ---
> Chun-Yeow
>
> On Tue, Jun 18, 2013 at 3:31 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
>> On Tue, 2013-06-18 at 12:07 +0800, Chun-Yeow Yeoh wrote:
>>> This is to support key deletion for mesh interface, especially
>>> to be used for key which is not deleted even with the deletion
>>> of peer mesh station.
>>
>> Can you explain which keys in mesh aren't deleted? It seems the
>> per-station keys would be deleted when the station is deleted, and
>> something like "GTK" would be deleted when leaving the mesh?
>>
>> johannes
>>

Re: [PATCH] mac80211: allow key deletion for mesh interface

[Yeoh Chun-Yeow]

In sta_info_get_bss, should it be:

if ((sta->sdata == sdata &&
                     (sta->sdata->bss && sta->sdata->bss == sdata->bss))

in stead of

if ((sta->sdata == sdata ||
                     (sta->sdata->bss && sta->sdata->bss == sdata->bss))

Otherwise, it seems that we still manage to get sta pointer even the
bss is NULL.

---
Chun-Yeow

On Tue, Jun 18, 2013 at 4:14 PM, Yeoh Chun-Yeow <yeohchunyeow@gmail.com> wrote:
> In the ieee80211_free_sta_keys, the key idx 4 is not deleted, since it
> is limited by NUM_DEFAULT_KEYS.
>
> ----
> Chun-Yeow
>
> On Tue, Jun 18, 2013 at 3:56 PM, Yeoh Chun-Yeow <yeohchunyeow@gmail.com> wrote:
>> In authsae, we have a GTK with key index 4 as follow:
>> install_key(&nlcfg, peer, CIPHER_AES_CMAC, NL80211_KEYTYPE_GROUP, 4, peer_mgtk);
>>
>> We may need to delete this key while doing re-authentication for the
>> same peer mesh STA due to accidental reboot. Otherwise, this GTK key
>> is not deleted.
>>
>> ---
>> Chun-Yeow
>>
>> On Tue, Jun 18, 2013 at 3:31 PM, Johannes Berg
>> <johannes@sipsolutions.net> wrote:
>>> On Tue, 2013-06-18 at 12:07 +0800, Chun-Yeow Yeoh wrote:
>>>> This is to support key deletion for mesh interface, especially
>>>> to be used for key which is not deleted even with the deletion
>>>> of peer mesh station.
>>>
>>> Can you explain which keys in mesh aren't deleted? It seems the
>>> per-station keys would be deleted when the station is deleted, and
>>> something like "GTK" would be deleted when leaving the mesh?
>>>
>>> johannes
>>>

Re: [PATCH] mac80211: allow key deletion for mesh interface

[Johannes Berg]

On Tue, 2013-06-18 at 17:07 +0800, Yeoh Chun-Yeow wrote:
> In sta_info_get_bss, should it be:
> 
> if ((sta->sdata == sdata &&
>                      (sta->sdata->bss && sta->sdata->bss == sdata->bss))
> 
> in stead of
> 
> if ((sta->sdata == sdata ||
>                      (sta->sdata->bss && sta->sdata->bss == sdata->bss))

What would the point be? Then we could just as well make it *only*
	sta->sdata == sdata
and it'd be the same as sta_info_get(). This is intentional for VLAN
handling.

> Otherwise, it seems that we still manage to get sta pointer even the
> bss is NULL.

What's the problem with that? But if this is a problem maybe we need to
explicitly check for non-NULL?

johannes

Re: [PATCH] mac80211: allow key deletion for mesh interface

[Johannes Berg]

On Tue, 2013-06-18 at 11:36 +0200, Johannes Berg wrote:

> > Otherwise, it seems that we still manage to get sta pointer even the
> > bss is NULL.
> 
> What's the problem with that? But if this is a problem maybe we need to
> explicitly check for non-NULL?

I should pay attention ... we already check for non-NULL, so I don't
understand what you're trying to say at all.

Maybe you're looking at old code?

johannes