Skip to contentSkip to conversation

fix buggy IEEE80211_CRYPT_* selects

PATCH In review [email protected] 4 participants 11 messages Mar 4, 2005 – Mar 9, 2005

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/3 fix buggy IEEE80211_CRYPT_* selects

From: Adrian Bunk <[email protected]>

Some of the options that needlessly wrote in their help text which options they do select (patch already sent) didn't obey the most important rule of select

If you select something, you have to ensure that the dependencies of what you do select are fulfilled.

resulting in the following compile error:

<-- snip -->

...
LD .tmp_vmlinux1
crypto/built-in.o(.init.text+0x31b): In function `aes_init':
: undefined reference to crypto_register_alg' crypto/built-in.o(.init.text+0x326): In function michael_mic_init':
: undefined reference to crypto_register_alg' crypto/built-in.o(.exit.text+0x6): In function aes_fini':
: undefined reference to crypto_unregister_alg' crypto/built-in.o(.exit.text+0x16): In function michael_mic_exit':
: undefined reference to crypto_unregister_alg' net/built-in.o(.text+0x5ba52): In function ieee80211_ccmp_init':
: undefined reference to crypto_alloc_tfm' net/built-in.o(.text+0x5ba94): In function ieee80211_ccmp_init':
: undefined reference to crypto_free_tfm' net/built-in.o(.text+0x5bab7): In function ieee80211_ccmp_deinit':
: undefined reference to crypto_free_tfm' net/built-in.o(.text+0x5c5c2): In function ieee80211_tkip_init':
: undefined reference to crypto_alloc_tfm' net/built-in.o(.text+0x5c5d5): In function ieee80211_tkip_init':
: undefined reference to crypto_alloc_tfm' net/built-in.o(.text+0x5c623): In function ieee80211_tkip_init':
: undefined reference to crypto_free_tfm' net/built-in.o(.text+0x5c62a): In function ieee80211_tkip_init':
: undefined reference to crypto_free_tfm' net/built-in.o(.text+0x5c65e): In function ieee80211_tkip_deinit':
: undefined reference to crypto_free_tfm' net/built-in.o(.text+0x5c665): In function ieee80211_tkip_deinit':
: undefined reference to `crypto_free_tfm'
make: *** [.tmp_vmlinux1] Error 1

<-- snip -->

This patch adds the missing selects of CRYPTO.

Signed-off-by Andrew Morton <[email protected]>

---

 25-akpm/net/ieee80211/Kconfig |    2 ++
 1 files changed, 2 insertions(+)
net/ieee80211/Kconfig renamed from net/ieee80211/Kconfig~fix-buggy-ieee80211_crypt_-selects +2 -0
diff -puN net/ieee80211/Kconfig~fix-buggy-ieee80211_crypt_-selects net/ieee80211/Kconfig
@@ -44,6 +44,7 @@ config IEEE80211_CRYPT_WEP
 config IEEE80211_CRYPT_CCMP
 	tristate "IEEE 802.11i CCMP support"
 	depends on IEEE80211
+	select CRYPTO
 	select CRYPTO_AES
 	---help---
 	Include software based cipher suites in support of IEEE 802.11i 

@@ -56,6 +57,7 @@ config IEEE80211_CRYPT_CCMP
 config IEEE80211_CRYPT_TKIP
 	tristate "IEEE 802.11i TKIP encryption"
 	depends on IEEE80211
+	select CRYPTO
 	select CRYPTO_MICHAEL_MIC
 	---help---
 	Include software based cipher suites in support of IEEE 802.11i

_

#1 lore raw
Quoted 21 lines · [email protected] wrote:

From: Adrian Bunk <[email protected]>

Some of the options that needlessly wrote in their help text which options they do select (patch already sent) didn't obey the most important rule of select

If you select something, you have to ensure that the dependencies of what you do select are fulfilled.

diff -puN net/ieee80211/Kconfig~fix-buggy-ieee80211_crypt_-selects net/ieee80211/Kconfig

net/ieee80211/Kconfig renamed from net/ieee80211/Kconfig~fix-buggy-ieee80211_crypt_-selects +1 -0
@@ -44,6 +44,7 @@ config IEEE80211_CRYPT_WEP
 config IEEE80211_CRYPT_CCMP
 	tristate "IEEE 802.11i CCMP support"
 	depends on IEEE80211
+	select CRYPTO
 	select CRYPTO_AES
 	---help---
 	Include software based cipher suites in support of IEEE 802.11i

net/ieee80211/Kconfig config IEEE80211_CRYPT_CCMP · line 62

Quoted 6 lines
net/ieee80211/Kconfig renamed from net/ieee80211/Kconfig~fix-buggy-ieee80211_crypt_-selects +1 -0
@@ -56,6 +57,7 @@ config IEEE80211_CRYPT_CCMP
 config IEEE80211_CRYPT_TKIP
 	tristate "IEEE 802.11i TKIP encryption"
 	depends on IEEE80211
+	select CRYPTO
 	select CRYPTO_MICHAEL_MIC
 	---help---

You are resending the old patch that is incorrect. We don't need multiple selects, CRYPTO_AES and CRYPTO_MICHAEL_MIC should pull things in.

Jeff

#2 lore raw
Quoted 26 lines · Jeff Garzik
Quoted 25 lines · [email protected] wrote:

From: Adrian Bunk <[email protected]>

Some of the options that needlessly wrote in their help text which options they do select (patch already sent) didn't obey the most important rule of select

If you select something, you have to ensure that the dependencies of what you do select are fulfilled.

diff -puN net/ieee80211/Kconfig~fix-buggy-ieee80211_crypt_-selects net/ieee80211/Kconfig
--- 25/net/ieee80211/Kconfig~fix-buggy-ieee80211_crypt_-selects 2005-02-28 14:49:54.000000000 -0800
+++ 25-akpm/net/ieee80211/Kconfig 2005-02-28 14:49:54.000000000 -0800

(quoted diff) +0 -0
@@ -44,6 +44,7 @@ config IEEE80211_CRYPT_WEP

config IEEE80211_CRYPT_CCMP
tristate "IEEE 802.11i CCMP support"
depends on IEEE80211

(quoted diff) +1 -0
+	select CRYPTO

	select CRYPTO_AES
	---help---
	Include software based cipher suites in support of IEEE 802.11i
(quoted diff) +0 -0
@@ -56,6 +57,7 @@ config IEEE80211_CRYPT_CCMP

config IEEE80211_CRYPT_TKIP
tristate "IEEE 802.11i TKIP encryption"

Quoted 8 lines
Quoted 4 lines

depends on IEEE80211

(quoted diff) +1 -0
+	select CRYPTO

	select CRYPTO_MICHAEL_MIC
	---help---

You are resending the old patch that is incorrect. We don't need multiple selects, CRYPTO_AES and CRYPTO_MICHAEL_MIC should pull things in.

As I already said, this implies that options like CRYPTO_AES and CRYPTO_MICHAEL_MIC can no longer depend on CRYPTO.

It's possible to change the crypto options in a way that CRYPTO is selected as required but no longer directly selectable (and I could send such a patch), but that's a decision of the crypto maintainers.

Quoted 1 line

Jeff

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
#3 lore raw
Quoted 37 lines · Adrian Bunk
Quoted 25 lines · Jeff Garzik
Quoted 23 lines · [email protected] wrote:

From: Adrian Bunk <[email protected]>

Some of the options that needlessly wrote in their help text which options they do select (patch already sent) didn't obey the most important rule of select

If you select something, you have to ensure that the dependencies of what you do select are fulfilled.

diff -puN net/ieee80211/Kconfig~fix-buggy-ieee80211_crypt_-selects net/ieee80211/Kconfig --- 25/net/ieee80211/Kconfig~fix-buggy-ieee80211_crypt_-selects 2005-02-28 14:49:54.000000000 -0800
+++ 25-akpm/net/ieee80211/Kconfig 2005-02-28 14:49:54.000000000 -0800

(quoted diff) +0 -0
@@ -44,6 +44,7 @@ config IEEE80211_CRYPT_WEP

config IEEE80211_CRYPT_CCMP
tristate "IEEE 802.11i CCMP support"
depends on IEEE80211

(quoted diff) +1 -0
+	select CRYPTO

	select CRYPTO_AES
	---help---
	Include software based cipher suites in support of IEEE 802.11i
(quoted diff) +0 -0
@@ -56,6 +57,7 @@ config IEEE80211_CRYPT_CCMP

config IEEE80211_CRYPT_TKIP

Quoted 8 lines
Quoted 5 lines

tristate "IEEE 802.11i TKIP encryption"
depends on IEEE80211

(quoted diff) +1 -0
+	select CRYPTO

	select CRYPTO_MICHAEL_MIC
	---help---

You are resending the old patch that is incorrect. We don't need multiple selects, CRYPTO_AES and CRYPTO_MICHAEL_MIC should pull things in.

Quoted 1 line

As I already said, this implies that options like CRYPTO_AES and CRYPTO_MICHAEL_MIC can no longer depend on CRYPTO.

No. Because CRYPTO_AES and CRYPTO_MICHAEL_MIC __obviously__ depend on CRYPTO, it should select CRYPTO automatically given the existing entries.

Otherwise, we must start specifying dependency chains in every damn Kconfig entry, which is completely illogical and a maintenance nightmare.

Jeff

#4 lore raw
Quoted 42 lines · Jeff Garzik
Quoted 41 lines · Adrian Bunk
Quoted 37 lines · Jeff Garzik
Quoted 29 lines · [email protected] wrote:

From: Adrian Bunk <[email protected]>

Some of the options that needlessly wrote in their help text which options
they do select (patch already sent) didn't obey the most important rule of
select

If you select something, you have to ensure that the dependencies of what you do select are fulfilled.

diff -puN net/ieee80211/Kconfig~fix-buggy-ieee80211_crypt_-selects net/ieee80211/Kconfig
--- 25/net/ieee80211/Kconfig~fix-buggy-ieee80211_crypt_-selects

2005-02-28 14:49:54.000000000 -0800

+++ 25-akpm/net/ieee80211/Kconfig 2005-02-28 14:49:54.000000000 -0800

(quoted diff) +0 -0
@@ -44,6 +44,7 @@ config IEEE80211_CRYPT_WEP

config IEEE80211_CRYPT_CCMP
tristate "IEEE 802.11i CCMP support"
depends on IEEE80211

(quoted diff) +1 -0
+	select CRYPTO

	select CRYPTO_AES
	---help---
	Include software based cipher suites in support of IEEE 802.11i
(quoted diff) +0 -0
@@ -56,6 +57,7 @@ config IEEE80211_CRYPT_CCMP

config IEEE80211_CRYPT_TKIP
tristate "IEEE 802.11i TKIP encryption"
depends on IEEE80211

(quoted diff) +1 -0
+	select CRYPTO
Quoted 2 lines
	select CRYPTO_MICHAEL_MIC
	---help---

You are resending the old patch that is incorrect. We don't need multiple selects, CRYPTO_AES and CRYPTO_MICHAEL_MIC should pull things in.

Quoted 8 lines
Quoted 2 lines

As I already said, this implies that options like CRYPTO_AES and CRYPTO_MICHAEL_MIC can no longer depend on CRYPTO.

No. Because CRYPTO_AES and CRYPTO_MICHAEL_MIC __obviously__ depend on CRYPTO, it should select CRYPTO automatically given the existing entries.

Otherwise, we must start specifying dependency chains in every damn Kconfig entry, which is completely illogical and a maintenance nightmare.

The way kconfig currently works, you have to ensure that the dependencies of what you select are fulfilled.

And handling dependencies as select chains creates some subtle problems:

Consider the following example:

config A

	tristate "foo"
	depends on !B && (C || D)

config E
tristate "bar"
select B

config F

	tristate "42"
	select A

With:
A=n
B=y
C=n
D=n
E=y

What values of the variables A-E do you expect exactly if the user turns on F?

Or even better, the code in question as a example due to the current confusing CRYPTO_AES dependencies (discussed in another thread and will be solved):

config IEEE80211_CRYPT_CCMP
tristate "IEEE 802.11i CCMP support"
depends on IEEE80211
select CRYPTO_AES

config CRYPTO_AES
tristate "AES cipher algorithms"
depends on CRYPTO && !(X86 && !X86_64)

On i386, should your "select CRYPTO_AES" set X86=n or X86_64=y ?

This would create even nastier bugs if it was hidden somewhere in a dependency chain.

Some people say that you mustn't select an user-visible option - this way you avoid any such problems.

This would mean you weren't allowed to select CRYPTO or CRYPTO_AES or CRYPTO_MICHAEL_MIC.

I'm less dogmatic regarding this because I care more about the usability of the kernel config system than about such rules, but what you want is simply not possible in a reasonable way.

Quoted 1 line

Jeff

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
#5 lore raw
Quoted 3 lines · Adrian Bunk

The way kconfig currently works, you have to ensure that the dependencies of what you select are fulfilled.

Yes Adrian's right. In the other places where we select CRYPTO symbols, we always make sure that CRYPTO itself is selected. See net/ipv4/Kconfig for example.

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#6 lore raw
Quoted 8 lines · Herbert Xu
Quoted 1 line · Adrian Bunk

The way kconfig currently works, you have to ensure that the dependencies of what you select are fulfilled.

Yes Adrian's right. In the other places where we select CRYPTO symbols, we always make sure that CRYPTO itself is selected. See net/ipv4/Kconfig for example.

I would rather fix Kconfig. If we are selecting X_1 -- which explicitly depends on X -- when Kconfig should automatically select X.

It is completely illogical to duplicate a dependency chain each time you wish to select a symbol.

Jeff

#7 lore raw
Quoted 11 lines · Jeff Garzik
Quoted 9 lines · Herbert Xu
Quoted 2 lines · Adrian Bunk

The way kconfig currently works, you have to ensure that the dependencies of what you select are fulfilled.

Yes Adrian's right. In the other places where we select CRYPTO symbols, we always make sure that CRYPTO itself is selected. See net/ipv4/Kconfig for example.

Quoted 5 lines

I would rather fix Kconfig. If we are selecting X_1 -- which explicitly depends on X -- when Kconfig should automatically select X.

It is completely illogical to duplicate a dependency chain each time you wish to select a symbol.

I asked about the first example of my last email:
What values of the variables A-E do you expect exactly if the user turns on F?

If you expect this to work, which unambiguous solution do you propose for this example?

Quoted 1 line

Jeff

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
#8 lore raw
Quoted 16 lines · Adrian Bunk
Quoted 14 lines · Jeff Garzik
Quoted 9 lines · Herbert Xu
Quoted 1 line · Adrian Bunk

The way kconfig currently works, you have to ensure that the dependencies of what you select are fulfilled.

Yes Adrian's right. In the other places where we select CRYPTO symbols, we always make sure that CRYPTO itself is selected. See net/ipv4/Kconfig for example.

I would rather fix Kconfig. If we are selecting X_1 -- which explicitly depends on X -- when Kconfig should automatically select X.

Quoted 8 lines
Quoted 1 line

It is completely illogical to duplicate a dependency chain each time you wish to select a symbol.

I asked about the first example of my last email:
What values of the variables A-E do you expect exactly if the user turns on F?

If you expect this to work, which unambiguous solution do you propose for this example?

If that information is missing, it should re-prompt the user, as it does for a few other cases.

Jeff

#9 lore raw
In reply to Jeff Garzik
Quoted 3 lines · Jeff Garzik

I would rather fix Kconfig. If we are selecting X_1 -- which explicitly depends on X -- when Kconfig should automatically select X.

That would be nice to have. However, until such a patch is integrated we need to write Kconfig entries that work properly.

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#10 lore raw
Quoted 7 lines · Herbert Xu
Quoted 1 line · Jeff Garzik

I would rather fix Kconfig. If we are selecting X_1 -- which explicitly depends on X -- when Kconfig should automatically select X.

That would be nice to have. However, until such a patch is integrated we need to write Kconfig entries that work properly.

Who has contacted the Kconfig maintainer, and asked about this?

Jeff