d80211: fix "iwconfig key [x]" behavior

PATCH Applied Hong Liu 2 participants 6 messages Aug 28, 2006 – Sep 22, 2006

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Hong Liu

author

2006-08-28 08:40 UTC #0 lore raw

PATCH 1/1 d80211: fix "iwconfig key [x]" behavior

"iwconfig key [x]" behavior is not correctly handled in the stack, also modify the giwencode method to show the key info.

Thanks,
Hong

[PATCH]d80211: fix "iwconfig key [x]" behavior

Signed-off-by Hong Liu <[email protected]>

net/d80211/ieee80211_ioctl.c +17 -8

index dd52555..d3dc59c 100644
@@ -2811,9 +2811,10 @@ static int ieee80211_ioctl_siwencode(str
 		if (sdata->default_key == NULL)
 			idx = 0;
 		else for (i = 0; i < NUM_DEFAULT_KEYS; i++) {
-			if (sdata->default_key == sdata->keys[i])
+			if (sdata->default_key == sdata->keys[i]) {
 				idx = i;
-			break;
+				break;
+			}
 		}
 		if (idx < 0)
 			return -EINVAL;

@@ -2824,16 +2825,22 @@ static int ieee80211_ioctl_siwencode(str
 		alg = ALG_NONE;
 	else if (erq->length == 0) {
 		/* No key data - just set the default TX key index */
-		sdata->default_key = sdata->keys[idx];
+		if (sdata->default_key != sdata->keys[idx]) {
+			if (sdata->default_key)
+				ieee80211_key_sysfs_remove_default(sdata);
+			sdata->default_key = sdata->keys[idx];
+			if (sdata->default_key)
+				ieee80211_key_sysfs_add_default(sdata);
+		}
+		return 0;
 	}
 
 	return ieee80211_set_encryption(
 		dev, bcaddr,
-		idx, erq->length == 0 ? ALG_NONE : ALG_WEP,
+		idx, alg,
 		sdata->default_key == NULL,
 		NULL, keybuf, erq->length);
 
-	return 0;
 }

@@ -2852,9 +2859,10 @@ static int ieee80211_ioctl_giwencode(str
 		if (sdata->default_key == NULL)
 			idx = 0;
 		else for (i = 0; i < NUM_DEFAULT_KEYS; i++) {
-			if (sdata->default_key == sdata->keys[i])
+			if (sdata->default_key == sdata->keys[i]) {
 				idx = i;
-			break;
+				break;
+			}
 		}
 		if (idx < 0)
 			return -EINVAL;

@@ -2869,7 +2877,8 @@ static int ieee80211_ioctl_giwencode(str
 		return 0;
 	}
 
-	erq->length = 0;
+	erq->length = sdata->keys[idx]->keylen;
+	memcpy(key, sdata->keys[idx]->key, erq->length);
 	erq->flags |= IW_ENCODE_ENABLED;
 
 	return 0;

Jiri Benc

2006-08-30 17:24 UTC #1 lore raw

Quoted 2 lines · Hong Liu

"iwconfig key [x]" behavior is not correctly handled in the stack, also modify the giwencode method to show the key info.

Thanks for spotting and fixing these bugs!

net/d80211/ieee80211_ioctl.c static int ieee80211_ioctl_giwencode(str · line 2881

Quoted 7 lines

net/d80211/ieee80211_ioctl.c +2 -1

@@ -2869,7 +2877,8 @@ static int ieee80211_ioctl_giwencode(str
 		return 0;
 	}
 
-	erq->length = 0;
+	erq->length = sdata->keys[idx]->keylen;
+	memcpy(key, sdata->keys[idx]->key, erq->length);

Shouldn't there be a check for buffer size here?

Thanks,

Jiri

Signature · 2 lines

Jiri Benc
SUSE Labs

Hong Liu

author

2006-09-01 03:42 UTC #2 lore raw

Quoted 16 lines · Jiri Benc

Quoted 2 lines · Hong Liu
"iwconfig key [x]" behavior is not correctly handled in the stack, also modify the giwencode method to show the key info.
Thanks for spotting and fixing these bugs!
Review static int ieee80211_ioctl_giwencode(str · line 2881
Quoted 7 lines
(quoted diff) +2 -1
@@ -2869,7 +2877,8 @@ static int ieee80211_ioctl_giwencode(str
              return 0;
      }
 
-     erq->length = 0;
+     erq->length = sdata->keys[idx]->keylen;
+     memcpy(key, sdata->keys[idx]->key, erq->length);
Shouldn't there be a check for buffer size here?

Quoted 9 lines

Thanks,
Jiri
--
Jiri Benc
SUSE Labs

Signed-off-by Hong Liu <[email protected]>

net/d80211/ieee80211_ioctl.c +20 ~~-11~~

index 89a58e3..c9a3524 100644
@@ -2867,9 +2867,10 @@ static int ieee80211_ioctl_siwencode(str
 		if (!sdata->default_key)
 			idx = 0;
 		else for (i = 0; i < NUM_DEFAULT_KEYS; i++) {
-			if (sdata->default_key == sdata->keys[i])
+			if (sdata->default_key == sdata->keys[i]) {
 				idx = i;
-			break;
+				break;
+			}
 		}
 		if (idx < 0)
 			return -EINVAL;

@@ -2880,16 +2881,21 @@ static int ieee80211_ioctl_siwencode(str
 		alg = ALG_NONE;
 	else if (erq->length == 0) {
 		/* No key data - just set the default TX key index */
-		sdata->default_key = sdata->keys[idx];
+		if (sdata->default_key != sdata->keys[idx]) {
+			if (sdata->default_key)
+				ieee80211_key_sysfs_remove_default(sdata);
+			sdata->default_key = sdata->keys[idx];
+			if (sdata->default_key)
+				ieee80211_key_sysfs_add_default(sdata);
+		}
+		return 0;
 	}
 
 	return ieee80211_set_encryption(
 		dev, bcaddr,
-		idx, erq->length == 0 ? ALG_NONE : ALG_WEP,
+		idx, alg,
 		!sdata->default_key,
 		NULL, keybuf, erq->length);
-
-	return 0;
 }

@@ -2908,9 +2914,10 @@ static int ieee80211_ioctl_giwencode(str
 		if (!sdata->default_key)
 			idx = 0;
 		else for (i = 0; i < NUM_DEFAULT_KEYS; i++) {
-			if (sdata->default_key == sdata->keys[i])
+			if (sdata->default_key == sdata->keys[i]) {
 				idx = i;
-			break;
+				break;
+			}
 		}
 		if (idx < 0)
 			return -EINVAL;

@@ -2925,7 +2932,8 @@ static int ieee80211_ioctl_giwencode(str
 		return 0;
 	}
 
-	erq->length = 0;
+	erq->length = min((int)erq->length, sdata->keys[idx]->keylen);
+	memcpy(key, sdata->keys[idx]->key, erq->length);
 	erq->flags |= IW_ENCODE_ENABLED;
 
 	return 0;

@@ -3054,9 +3062,10 @@ static int ieee80211_ioctl_siwencodeext(
 		if (!sdata->default_key)
 			idx = 0;
 		else for (i = 0; i < NUM_DEFAULT_KEYS; i++) {
-			if (sdata->default_key == sdata->keys[i])
+			if (sdata->default_key == sdata->keys[i]) {
 				idx = i;
-			break;
+				break;
+			}
 		}
 		if (idx < 0)
 			return -EINVAL;

Jiri Benc

2006-09-21 15:51 UTC #3 lore raw

Hi,

sorry for the delay.

net/d80211/ieee80211_ioctl.c static int ieee80211_ioctl_giwencode(str · line 2939

Quoted 10 lines · Hong Liu

net/d80211/ieee80211_ioctl.c +2 -1

@@ -2925,7 +2932,8 @@ static int ieee80211_ioctl_giwencode(str
 		return 0;
 	}
 
-	erq->length = 0;
+	erq->length = min((int)erq->length, sdata->keys[idx]->keylen);
+	memcpy(key, sdata->keys[idx]->key, erq->length);
 	erq->flags |= IW_ENCODE_ENABLED;
 
 	return 0;

This is still not correct, erq->length should be set to the actual key length to allow WE to return -E2BIG in case of too small buffer.

	memcpy(key, sdata->keys[idx]->key,
	       min((int)erq->length, sdata->keys[idx]->keylen));
	erq->length = sdata->keys[idx]->keylen;

Jiri

Signature · 2 lines

Jiri Benc
SUSE Labs

Hong Liu

author

2006-09-22 08:03 UTC #4 lore raw

Quoted 19 lines · Jiri Benc

Hi,
sorry for the delay.
Review static int ieee80211_ioctl_giwencode(str · line 2939
Quoted 10 lines · Hong Liu
(quoted diff) +2 -1
@@ -2925,7 +2932,8 @@ static int ieee80211_ioctl_giwencode(str
 		return 0;
 	}
 
-	erq->length = 0;
+	erq->length = min((int)erq->length, sdata->keys[idx]->keylen);
+	memcpy(key, sdata->keys[idx]->key, erq->length);
 	erq->flags |= IW_ENCODE_ENABLED;
 
 	return 0;
This is still not correct, erq->length should be set to the actual key length to allow WE to return -E2BIG in case of too small buffer.

Quoted 5 lines

	memcpy(key, sdata->keys[idx]->key,
	       min((int)erq->length, sdata->keys[idx]->keylen));
	erq->length = sdata->keys[idx]->keylen;

Jiri

Resend.

Thanks,
Hong

Signed-off-by Hong Liu <[email protected]>

net/d80211/ieee80211_ioctl.c +21 ~~-11~~

index 89a58e3..bde0c43 100644
@@ -2867,9 +2867,10 @@ static int ieee80211_ioctl_siwencode(str
 		if (!sdata->default_key)
 			idx = 0;
 		else for (i = 0; i < NUM_DEFAULT_KEYS; i++) {
-			if (sdata->default_key == sdata->keys[i])
+			if (sdata->default_key == sdata->keys[i]) {
 				idx = i;
-			break;
+				break;
+			}
 		}
 		if (idx < 0)
 			return -EINVAL;

@@ -2880,16 +2881,21 @@ static int ieee80211_ioctl_siwencode(str
 		alg = ALG_NONE;
 	else if (erq->length == 0) {
 		/* No key data - just set the default TX key index */
-		sdata->default_key = sdata->keys[idx];
+		if (sdata->default_key != sdata->keys[idx]) {
+			if (sdata->default_key)
+				ieee80211_key_sysfs_remove_default(sdata);
+			sdata->default_key = sdata->keys[idx];
+			if (sdata->default_key)
+				ieee80211_key_sysfs_add_default(sdata);
+		}
+		return 0;
 	}
 
 	return ieee80211_set_encryption(
 		dev, bcaddr,
-		idx, erq->length == 0 ? ALG_NONE : ALG_WEP,
+		idx, alg,
 		!sdata->default_key,
 		NULL, keybuf, erq->length);
-
-	return 0;
 }

@@ -2908,9 +2914,10 @@ static int ieee80211_ioctl_giwencode(str
 		if (!sdata->default_key)
 			idx = 0;
 		else for (i = 0; i < NUM_DEFAULT_KEYS; i++) {
-			if (sdata->default_key == sdata->keys[i])
+			if (sdata->default_key == sdata->keys[i]) {
 				idx = i;
-			break;
+				break;
+			}
 		}
 		if (idx < 0)
 			return -EINVAL;

@@ -2925,7 +2932,9 @@ static int ieee80211_ioctl_giwencode(str
 		return 0;
 	}
 
-	erq->length = 0;
+	memcpy(key, sdata->keys[idx]->key,
+	       min((int)erq->length, sdata->keys[idx]->keylen));
+	erq->length = sdata->keys[idx]->keylen;
 	erq->flags |= IW_ENCODE_ENABLED;
 
 	return 0;

@@ -3054,9 +3063,10 @@ static int ieee80211_ioctl_siwencodeext(
 		if (!sdata->default_key)
 			idx = 0;
 		else for (i = 0; i < NUM_DEFAULT_KEYS; i++) {
-			if (sdata->default_key == sdata->keys[i])
+			if (sdata->default_key == sdata->keys[i]) {
 				idx = i;
-			break;
+				break;
+			}
 		}
 		if (idx < 0)
 			return -EINVAL;

Jiri Benc

Applied

2006-09-22 12:00 UTC #5 lore raw

Quoted 1 line · Hong Liu

Resend.

Applied to my tree, thanks for the patch!

Jiri

Signature · 2 lines

Jiri Benc
SUSE Labs