[PATCH] cfg80211: avoid setting default_key if add_key fails

[PATCH] cfg80211: avoid setting default_key if add_key fails

[Zhu Yi]

In cfg80211_upload_connect_keys(), we call add_key, set_default_key
and set_default_mgmt_key (if applicable) one by one. If one of these
operations fails, we should stop calling the following functions.
Because if the key is not added successfully, we should not set it as
default key anyway.

Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
 net/wireless/util.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/wireless/util.c b/net/wireless/util.c
index 4bab380..ba387d8 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -546,13 +546,17 @@ void cfg80211_upload_connect_keys(struct wireless_dev *wdev)
 		if (!wdev->connect_keys->params[i].cipher)
 			continue;
 		if (rdev->ops->add_key(wdev->wiphy, dev, i, NULL,
-					&wdev->connect_keys->params[i]))
+					&wdev->connect_keys->params[i])) {
 			printk(KERN_ERR "%s: failed to set key %d\n",
 				dev->name, i);
+			continue;
+		}
 		if (wdev->connect_keys->def == i)
-			if (rdev->ops->set_default_key(wdev->wiphy, dev, i))
+			if (rdev->ops->set_default_key(wdev->wiphy, dev, i)) {
 				printk(KERN_ERR "%s: failed to set defkey %d\n",
 					dev->name, i);
+				continue;
+			}
 		if (wdev->connect_keys->defmgmt == i)
 			if (rdev->ops->set_default_mgmt_key(wdev->wiphy, dev, i))
 				printk(KERN_ERR "%s: failed to set mgtdef %d\n",
-- 
1.6.0.4

Re: [PATCH] cfg80211: avoid setting default_key if add_key fails

[Johannes Berg]

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

On Mon, 2009-07-20 at 16:12 +0800, Zhu Yi wrote:
> In cfg80211_upload_connect_keys(), we call add_key, set_default_key
> and set_default_mgmt_key (if applicable) one by one. If one of these
> operations fails, we should stop calling the following functions.
> Because if the key is not added successfully, we should not set it as
> default key anyway.

Should we even disconnect again in that case?

johannes

> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
> ---
>  net/wireless/util.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/net/wireless/util.c b/net/wireless/util.c
> index 4bab380..ba387d8 100644
> --- a/net/wireless/util.c
> +++ b/net/wireless/util.c
> @@ -546,13 +546,17 @@ void cfg80211_upload_connect_keys(struct wireless_dev *wdev)
>  		if (!wdev->connect_keys->params[i].cipher)
>  			continue;
>  		if (rdev->ops->add_key(wdev->wiphy, dev, i, NULL,
> -					&wdev->connect_keys->params[i]))
> +					&wdev->connect_keys->params[i])) {
>  			printk(KERN_ERR "%s: failed to set key %d\n",
>  				dev->name, i);
> +			continue;
> +		}
>  		if (wdev->connect_keys->def == i)
> -			if (rdev->ops->set_default_key(wdev->wiphy, dev, i))
> +			if (rdev->ops->set_default_key(wdev->wiphy, dev, i)) {
>  				printk(KERN_ERR "%s: failed to set defkey %d\n",
>  					dev->name, i);
> +				continue;
> +			}
>  		if (wdev->connect_keys->defmgmt == i)
>  			if (rdev->ops->set_default_mgmt_key(wdev->wiphy, dev, i))
>  				printk(KERN_ERR "%s: failed to set mgtdef %d\n",

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

Re: [PATCH] cfg80211: avoid setting default_key if add_key fails

[Zhu Yi]

On Mon, 2009-07-20 at 18:34 +0800, Johannes Berg wrote:
> On Mon, 2009-07-20 at 16:12 +0800, Zhu Yi wrote:
> > In cfg80211_upload_connect_keys(), we call add_key, set_default_key
> > and set_default_mgmt_key (if applicable) one by one. If one of these
> > operations fails, we should stop calling the following functions.
> > Because if the key is not added successfully, we should not set it as
> > default key anyway.
> 
> Should we even disconnect again in that case?

Yeah, if one key setting is failed, it might not be worth trying others.
This is true for iwmc3200wifi. But I cannot speak for other drivers. How
about merge this patch first? This is clearly a bug because the firmware
confused when it is told to set a key as default but never existed.

Thanks,
-yi

> > Signed-off-by: Zhu Yi <yi.zhu@intel.com>
> > ---
> >  net/wireless/util.c |    8 ++++++--
> >  1 files changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/wireless/util.c b/net/wireless/util.c
> > index 4bab380..ba387d8 100644
> > --- a/net/wireless/util.c
> > +++ b/net/wireless/util.c
> > @@ -546,13 +546,17 @@ void cfg80211_upload_connect_keys(struct wireless_dev *wdev)
> >  		if (!wdev->connect_keys->params[i].cipher)
> >  			continue;
> >  		if (rdev->ops->add_key(wdev->wiphy, dev, i, NULL,
> > -					&wdev->connect_keys->params[i]))
> > +					&wdev->connect_keys->params[i])) {
> >  			printk(KERN_ERR "%s: failed to set key %d\n",
> >  				dev->name, i);
> > +			continue;
> > +		}
> >  		if (wdev->connect_keys->def == i)
> > -			if (rdev->ops->set_default_key(wdev->wiphy, dev, i))
> > +			if (rdev->ops->set_default_key(wdev->wiphy, dev, i)) {
> >  				printk(KERN_ERR "%s: failed to set defkey %d\n",
> >  					dev->name, i);
> > +				continue;
> > +			}
> >  		if (wdev->connect_keys->defmgmt == i)
> >  			if (rdev->ops->set_default_mgmt_key(wdev->wiphy, dev, i))
> >  				printk(KERN_ERR "%s: failed to set mgtdef %d\n",

Re: [PATCH] cfg80211: avoid setting default_key if add_key fails

[Johannes Berg]

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

On Tue, 2009-07-21 at 09:47 +0800, Zhu Yi wrote:

> > Should we even disconnect again in that case?
> 
> Yeah, if one key setting is failed, it might not be worth trying others.

Well, even if the others don't fail, the connection wouldn't be working
properly in that case.

> This is true for iwmc3200wifi. But I cannot speak for other drivers. How
> about merge this patch first? This is clearly a bug because the firmware
> confused when it is told to set a key as default but never existed.

Agree, let's merge this first and then see how to disconnect.

johannes

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