[patch 1/3] fix buggy IEEE80211_CRYPT_* selects

[patch 1/3] fix buggy IEEE80211_CRYPT_* selects

[akpm]

From: Adrian Bunk <bunk@stusta.de>

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 <akpm@osdl.org>
---

 25-akpm/net/ieee80211/Kconfig |    2 ++
 1 files changed, 2 insertions(+)

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
@@ -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 
_

Re: [patch 1/3] fix buggy IEEE80211_CRYPT_* selects

[Jeff Garzik]

akpm@osdl.org wrote:
> From: Adrian Bunk <bunk@stusta.de>
> 
> 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
> @@ -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---


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

Re: [patch 1/3] fix buggy IEEE80211_CRYPT_* selects

[Adrian Bunk]

On Fri, Mar 04, 2005 at 12:33:19PM -0500, Jeff Garzik wrote:
> akpm@osdl.org wrote:
> >From: Adrian Bunk <bunk@stusta.de>
> >
> >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
> >@@ -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---
> 
> 
> 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.

> 	Jeff

cu
Adrian

-- 

       "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

Re: [patch 1/3] fix buggy IEEE80211_CRYPT_* selects

[Jeff Garzik]

Adrian Bunk wrote:
> On Fri, Mar 04, 2005 at 12:33:19PM -0500, Jeff Garzik wrote:
> 
>>akpm@osdl.org wrote:
>>
>>>From: Adrian Bunk <bunk@stusta.de>
>>>
>>>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
>>>@@ -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---
>>
>>
>>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.

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

Re: [patch 1/3] fix buggy IEEE80211_CRYPT_* selects

[Adrian Bunk]

On Fri, Mar 04, 2005 at 05:21:22PM -0500, Jeff Garzik wrote:
> Adrian Bunk wrote:
> >On Fri, Mar 04, 2005 at 12:33:19PM -0500, Jeff Garzik wrote:
> >
> >>akpm@osdl.org wrote:
> >>
> >>>From: Adrian Bunk <bunk@stusta.de>
> >>>
> >>>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
> >>>@@ -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---
> >>
> >>
> >>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.
> 
> 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.


> 	Jeff

cu
Adrian

-- 

       "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

Re: [patch 1/3] fix buggy IEEE80211_CRYPT_* selects

[Herbert Xu]

On Sat, Mar 05, 2005 at 12:07:18AM +0100, Adrian Bunk wrote:
> 
> 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,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [patch 1/3] fix buggy IEEE80211_CRYPT_* selects

[Jeff Garzik]

Herbert Xu wrote:
> On Sat, Mar 05, 2005 at 12:07:18AM +0100, Adrian Bunk wrote:
> 
>>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

Re: [patch 1/3] fix buggy IEEE80211_CRYPT_* selects

[Adrian Bunk]

On Sun, Mar 06, 2005 at 01:00:50PM -0500, Jeff Garzik wrote:
> Herbert Xu wrote:
> >On Sat, Mar 05, 2005 at 12:07:18AM +0100, Adrian Bunk wrote:
> >
> >>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.

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?

> 	Jeff

cu
Adrian

-- 

       "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

Re: [patch 1/3] fix buggy IEEE80211_CRYPT_* selects

[Jeff Garzik]

Adrian Bunk wrote:
> On Sun, Mar 06, 2005 at 01:00:50PM -0500, Jeff Garzik wrote:
> 
>>Herbert Xu wrote:
>>
>>>On Sat, Mar 05, 2005 at 12:07:18AM +0100, Adrian Bunk wrote:
>>>
>>>
>>>>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.
> 
> 
> 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

Re: [patch 1/3] fix buggy IEEE80211_CRYPT_* selects

[Herbert Xu]

On Sun, Mar 06, 2005 at 01:00:50PM -0500, Jeff Garzik wrote:
> 
> 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,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [patch 1/3] fix buggy IEEE80211_CRYPT_* selects

[Jeff Garzik]

Herbert Xu wrote:
> On Sun, Mar 06, 2005 at 01:00:50PM -0500, Jeff Garzik wrote:
> 
>>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