* [PATCH] staging: ks7010: select CRYPTO_HASH/CRYPTO_MICHAEL_MIC @ 2021-10-11 15:29 Vegard Nossum 2021-10-11 15:37 ` Greg Kroah-Hartman 2021-10-13 12:26 ` Greg Kroah-Hartman 0 siblings, 2 replies; 7+ messages in thread From: Vegard Nossum @ 2021-10-11 15:29 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Arnd Bergmann, linux-staging, linux-kernel, Vegard Nossum Fix the following build/link errors: ld: drivers/staging/ks7010/ks_hostif.o: in function `michael_mic.constprop.0': ks_hostif.c:(.text+0x95b): undefined reference to `crypto_alloc_shash' ld: ks_hostif.c:(.text+0x97a): undefined reference to `crypto_shash_setkey' ld: ks_hostif.c:(.text+0xa13): undefined reference to `crypto_shash_update' ld: ks_hostif.c:(.text+0xa28): undefined reference to `crypto_shash_update' ld: ks_hostif.c:(.text+0xa48): undefined reference to `crypto_shash_finup' ld: ks_hostif.c:(.text+0xa6d): undefined reference to `crypto_destroy_tfm' Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> --- drivers/staging/ks7010/Kconfig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/ks7010/Kconfig b/drivers/staging/ks7010/Kconfig index 0987fdc2f70db..8ea6c09286798 100644 --- a/drivers/staging/ks7010/Kconfig +++ b/drivers/staging/ks7010/Kconfig @@ -5,6 +5,9 @@ config KS7010 select WIRELESS_EXT select WEXT_PRIV select FW_LOADER + select CRYPTO + select CRYPTO_HASH + select CRYPTO_MICHAEL_MIC help This is a driver for KeyStream KS7010 based SDIO WIFI cards. It is found on at least later Spectec SDW-821 (FCC-ID "S2Y-WLAN-11G-K" only, -- 2.23.0.718.g5ad94255a8 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: ks7010: select CRYPTO_HASH/CRYPTO_MICHAEL_MIC 2021-10-11 15:29 [PATCH] staging: ks7010: select CRYPTO_HASH/CRYPTO_MICHAEL_MIC Vegard Nossum @ 2021-10-11 15:37 ` Greg Kroah-Hartman 2021-10-11 16:55 ` Vegard Nossum 2021-10-13 12:26 ` Greg Kroah-Hartman 1 sibling, 1 reply; 7+ messages in thread From: Greg Kroah-Hartman @ 2021-10-11 15:37 UTC (permalink / raw) To: Vegard Nossum; +Cc: Arnd Bergmann, linux-staging, linux-kernel On Mon, Oct 11, 2021 at 05:29:41PM +0200, Vegard Nossum wrote: > Fix the following build/link errors: > > ld: drivers/staging/ks7010/ks_hostif.o: in function `michael_mic.constprop.0': > ks_hostif.c:(.text+0x95b): undefined reference to `crypto_alloc_shash' > ld: ks_hostif.c:(.text+0x97a): undefined reference to `crypto_shash_setkey' > ld: ks_hostif.c:(.text+0xa13): undefined reference to `crypto_shash_update' > ld: ks_hostif.c:(.text+0xa28): undefined reference to `crypto_shash_update' > ld: ks_hostif.c:(.text+0xa48): undefined reference to `crypto_shash_finup' > ld: ks_hostif.c:(.text+0xa6d): undefined reference to `crypto_destroy_tfm' > > Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> > --- > drivers/staging/ks7010/Kconfig | 3 +++ > 1 file changed, 3 insertions(+) What commit caused these issues? thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: ks7010: select CRYPTO_HASH/CRYPTO_MICHAEL_MIC 2021-10-11 15:37 ` Greg Kroah-Hartman @ 2021-10-11 16:55 ` Vegard Nossum 0 siblings, 0 replies; 7+ messages in thread From: Vegard Nossum @ 2021-10-11 16:55 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Arnd Bergmann, linux-staging, linux-kernel, Stephen Rothwell, YueHaibing On 10/11/21 5:37 PM, Greg Kroah-Hartman wrote: > On Mon, Oct 11, 2021 at 05:29:41PM +0200, Vegard Nossum wrote: >> Fix the following build/link errors: >> >> ld: drivers/staging/ks7010/ks_hostif.o: in function `michael_mic.constprop.0': >> ks_hostif.c:(.text+0x95b): undefined reference to `crypto_alloc_shash' >> ld: ks_hostif.c:(.text+0x97a): undefined reference to `crypto_shash_setkey' >> ld: ks_hostif.c:(.text+0xa13): undefined reference to `crypto_shash_update' >> ld: ks_hostif.c:(.text+0xa28): undefined reference to `crypto_shash_update' >> ld: ks_hostif.c:(.text+0xa48): undefined reference to `crypto_shash_finup' >> ld: ks_hostif.c:(.text+0xa6d): undefined reference to `crypto_destroy_tfm' >> >> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> >> --- >> drivers/staging/ks7010/Kconfig | 3 +++ >> 1 file changed, 3 insertions(+) > > What commit caused these issues? > > thanks, > > greg k-h > So I found this commit that appears to (attempt to) fix this issue already: commit 3e5bc68fa596874500e8c718605aa44d5e42a34c Author: YueHaibing <yuehaibing@huawei.com> Date: Fri Jun 21 15:24:55 2019 +0800 staging: ks7010: Fix build error when CRYPTO is m and KS7010 is y, building fails: drivers/staging/ks7010/ks_hostif.o: In function `michael_mic.constprop.13': ks_hostif.c:(.text+0x560): undefined reference to `crypto_alloc_shash' ks_hostif.c:(.text+0x580): undefined reference to `crypto_shash_setkey' ks_hostif.c:(.text+0x5e0): undefined reference to `crypto_destroy_tfm' ks_hostif.c:(.text+0x614): undefined reference to `crypto_shash_update' ks_hostif.c:(.text+0x62c): undefined reference to `crypto_shash_update' ks_hostif.c:(.text+0x648): undefined reference to `crypto_shash_finup' Add CRYPTO and CRYPTO_HASH dependencies to fix this. Reported-by: Hulk Robot <hulkci@huawei.com> Fixes: 8b523f20417d ("staging: ks7010: removed custom Michael MIC implementation.") Signed-off-by: YueHaibing <yuehaibing@huawei.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Notes (mainline): Fixes: 8b523f20417d ("staging: ks7010: removed custom Michael MIC implementation.") # v5.1-rc1 Lore: https://lore.kernel.org/r/20190621034221.36708-1-yuehaibing@huawei.com # driverdev-devel, lkml However, it was reverted in v5.2-rc6: commit a4961427e74948ced9ddd40f3efb2a206b87e4c8 Author: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Date: Mon Jun 24 16:45:34 2019 +0800 Revert "staging: ks7010: Fix build error" This reverts commit 3e5bc68fa596874500e8c718605aa44d5e42a34c as it causes build errors in linux-next. Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Cc: YueHaibing <yuehaibing@huawei.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Looks like this was the build error in question: https://lore.kernel.org/all/20190624173855.3c188955@canb.auug.org.au/ To be honest I don't really see what the full recursive dependency cycle is, but I think "select" should work instead -- Arnd? Maybe we can add this: Fixes: 8b523f20417d ("staging: ks7010: removed custom Michael MIC implementation.") Fixes: 3e5bc68fa5968 ("staging: ks7010: Fix build error") Fixes: a4961427e7494 ("Revert "staging: ks7010: Fix build error"") Vegard ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: ks7010: select CRYPTO_HASH/CRYPTO_MICHAEL_MIC 2021-10-11 15:29 [PATCH] staging: ks7010: select CRYPTO_HASH/CRYPTO_MICHAEL_MIC Vegard Nossum 2021-10-11 15:37 ` Greg Kroah-Hartman @ 2021-10-13 12:26 ` Greg Kroah-Hartman 2021-10-13 12:51 ` Vegard Nossum 1 sibling, 1 reply; 7+ messages in thread From: Greg Kroah-Hartman @ 2021-10-13 12:26 UTC (permalink / raw) To: Vegard Nossum; +Cc: Arnd Bergmann, linux-staging, linux-kernel On Mon, Oct 11, 2021 at 05:29:41PM +0200, Vegard Nossum wrote: > Fix the following build/link errors: > > ld: drivers/staging/ks7010/ks_hostif.o: in function `michael_mic.constprop.0': > ks_hostif.c:(.text+0x95b): undefined reference to `crypto_alloc_shash' > ld: ks_hostif.c:(.text+0x97a): undefined reference to `crypto_shash_setkey' > ld: ks_hostif.c:(.text+0xa13): undefined reference to `crypto_shash_update' > ld: ks_hostif.c:(.text+0xa28): undefined reference to `crypto_shash_update' > ld: ks_hostif.c:(.text+0xa48): undefined reference to `crypto_shash_finup' > ld: ks_hostif.c:(.text+0xa6d): undefined reference to `crypto_destroy_tfm' > > Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> > --- > drivers/staging/ks7010/Kconfig | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/staging/ks7010/Kconfig b/drivers/staging/ks7010/Kconfig > index 0987fdc2f70db..8ea6c09286798 100644 > --- a/drivers/staging/ks7010/Kconfig > +++ b/drivers/staging/ks7010/Kconfig > @@ -5,6 +5,9 @@ config KS7010 > select WIRELESS_EXT > select WEXT_PRIV > select FW_LOADER > + select CRYPTO > + select CRYPTO_HASH > + select CRYPTO_MICHAEL_MIC Let's try to rely on 'depend' and not 'select' please. thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: ks7010: select CRYPTO_HASH/CRYPTO_MICHAEL_MIC 2021-10-13 12:26 ` Greg Kroah-Hartman @ 2021-10-13 12:51 ` Vegard Nossum 2021-10-13 13:03 ` Arnd Bergmann 0 siblings, 1 reply; 7+ messages in thread From: Vegard Nossum @ 2021-10-13 12:51 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Arnd Bergmann, linux-staging, linux-kernel, Randy Dunlap, Herbert Xu, David S. Miller, linux-crypto On 10/13/21 2:26 PM, Greg Kroah-Hartman wrote: > On Mon, Oct 11, 2021 at 05:29:41PM +0200, Vegard Nossum wrote: >> Fix the following build/link errors: >> >> ld: drivers/staging/ks7010/ks_hostif.o: in function `michael_mic.constprop.0': >> ks_hostif.c:(.text+0x95b): undefined reference to `crypto_alloc_shash' >> ld: ks_hostif.c:(.text+0x97a): undefined reference to `crypto_shash_setkey' >> ld: ks_hostif.c:(.text+0xa13): undefined reference to `crypto_shash_update' >> ld: ks_hostif.c:(.text+0xa28): undefined reference to `crypto_shash_update' >> ld: ks_hostif.c:(.text+0xa48): undefined reference to `crypto_shash_finup' >> ld: ks_hostif.c:(.text+0xa6d): undefined reference to `crypto_destroy_tfm' >> >> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> >> --- >> drivers/staging/ks7010/Kconfig | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/staging/ks7010/Kconfig b/drivers/staging/ks7010/Kconfig >> index 0987fdc2f70db..8ea6c09286798 100644 >> --- a/drivers/staging/ks7010/Kconfig >> +++ b/drivers/staging/ks7010/Kconfig >> @@ -5,6 +5,9 @@ config KS7010 >> select WIRELESS_EXT >> select WEXT_PRIV >> select FW_LOADER >> + select CRYPTO >> + select CRYPTO_HASH >> + select CRYPTO_MICHAEL_MIC > > Let's try to rely on 'depend' and not 'select' please. I used 'select' because it seemed to be the established pattern for these options. Compare: $ find -name '*Kconfig*' | xargs git grep 'depends on CRYPTO$' | wc --lines 1 $ find -name '*Kconfig*' | xargs git grep 'select CRYPTO$' | wc --lines 66 $ find -name '*Kconfig*' | xargs git grep 'depends on CRYPTO' | wc --lines 87 $ find -name '*Kconfig*' | xargs git grep 'select CRYPTO' | wc --lines 1005 That said, I have found several other cases where CRYPTO_* algorithms are getting 'select'-ed without also selecting CRYPTO/CRYPTO_HASH, so I definitely see the problem you're trying to address. I've added some more people on Cc to see if there is a consensus on the best way to do this for the CRYPTO* options going forwards. Thoughts, anybody? Vegard ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: ks7010: select CRYPTO_HASH/CRYPTO_MICHAEL_MIC 2021-10-13 12:51 ` Vegard Nossum @ 2021-10-13 13:03 ` Arnd Bergmann 2021-10-13 13:11 ` Greg Kroah-Hartman 0 siblings, 1 reply; 7+ messages in thread From: Arnd Bergmann @ 2021-10-13 13:03 UTC (permalink / raw) To: Vegard Nossum Cc: Greg Kroah-Hartman, Arnd Bergmann, linux-staging, Linux Kernel Mailing List, Randy Dunlap, Herbert Xu, David S. Miller, open list:HARDWARE RANDOM NUMBER GENERATOR CORE On Wed, Oct 13, 2021 at 2:51 PM Vegard Nossum <vegard.nossum@oracle.com> wrote: > On 10/13/21 2:26 PM, Greg Kroah-Hartman wrote: > > On Mon, Oct 11, 2021 at 05:29:41PM +0200, Vegard Nossum wrote: > >> Fix the following build/link errors: > >> > >> ld: drivers/staging/ks7010/ks_hostif.o: in function `michael_mic.constprop.0': > >> ks_hostif.c:(.text+0x95b): undefined reference to `crypto_alloc_shash' > >> ld: ks_hostif.c:(.text+0x97a): undefined reference to `crypto_shash_setkey' > >> ld: ks_hostif.c:(.text+0xa13): undefined reference to `crypto_shash_update' > >> ld: ks_hostif.c:(.text+0xa28): undefined reference to `crypto_shash_update' > >> ld: ks_hostif.c:(.text+0xa48): undefined reference to `crypto_shash_finup' > >> ld: ks_hostif.c:(.text+0xa6d): undefined reference to `crypto_destroy_tfm' > >> > >> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> > >> --- > >> drivers/staging/ks7010/Kconfig | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/drivers/staging/ks7010/Kconfig b/drivers/staging/ks7010/Kconfig > >> index 0987fdc2f70db..8ea6c09286798 100644 > >> --- a/drivers/staging/ks7010/Kconfig > >> +++ b/drivers/staging/ks7010/Kconfig > >> @@ -5,6 +5,9 @@ config KS7010 > >> select WIRELESS_EXT > >> select WEXT_PRIV > >> select FW_LOADER > >> + select CRYPTO > >> + select CRYPTO_HASH > >> + select CRYPTO_MICHAEL_MIC > > > > Let's try to rely on 'depend' and not 'select' please. > > I used 'select' because it seemed to be the established pattern for > these options. Yes, this is the correct way to do it here. In general, using "depends on" is better than "select", especially when crossing subsystem boundaries, however mixing the two is what really hurts because of the circular dependencies you'd get. The only way we could use 'depends on' here would be to change all the other drivers to do the same. > Compare: > > $ find -name '*Kconfig*' | xargs git grep 'depends on CRYPTO$' | wc --lines > 1 > > $ find -name '*Kconfig*' | xargs git grep 'select CRYPTO$' | wc --lines > 66 > > $ find -name '*Kconfig*' | xargs git grep 'depends on CRYPTO' | wc --lines > 87 > > $ find -name '*Kconfig*' | xargs git grep 'select CRYPTO' | wc --lines > 1005 > > That said, I have found several other cases where CRYPTO_* algorithms > are getting 'select'-ed without also selecting CRYPTO/CRYPTO_HASH, so I > definitely see the problem you're trying to address. > > I've added some more people on Cc to see if there is a consensus on the > best way to do this for the CRYPTO* options going forwards. Thoughts, > anybody? I don't think there is much point in trying to change the existing pattern for crypto. We might want to change some of those that use 'depends on' at the moment for consistency, though most of those look correct as well. Arnd ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: ks7010: select CRYPTO_HASH/CRYPTO_MICHAEL_MIC 2021-10-13 13:03 ` Arnd Bergmann @ 2021-10-13 13:11 ` Greg Kroah-Hartman 0 siblings, 0 replies; 7+ messages in thread From: Greg Kroah-Hartman @ 2021-10-13 13:11 UTC (permalink / raw) To: Arnd Bergmann Cc: Vegard Nossum, linux-staging, Linux Kernel Mailing List, Randy Dunlap, Herbert Xu, David S. Miller, open list:HARDWARE RANDOM NUMBER GENERATOR CORE On Wed, Oct 13, 2021 at 03:03:41PM +0200, Arnd Bergmann wrote: > On Wed, Oct 13, 2021 at 2:51 PM Vegard Nossum <vegard.nossum@oracle.com> wrote: > > On 10/13/21 2:26 PM, Greg Kroah-Hartman wrote: > > > On Mon, Oct 11, 2021 at 05:29:41PM +0200, Vegard Nossum wrote: > > >> Fix the following build/link errors: > > >> > > >> ld: drivers/staging/ks7010/ks_hostif.o: in function `michael_mic.constprop.0': > > >> ks_hostif.c:(.text+0x95b): undefined reference to `crypto_alloc_shash' > > >> ld: ks_hostif.c:(.text+0x97a): undefined reference to `crypto_shash_setkey' > > >> ld: ks_hostif.c:(.text+0xa13): undefined reference to `crypto_shash_update' > > >> ld: ks_hostif.c:(.text+0xa28): undefined reference to `crypto_shash_update' > > >> ld: ks_hostif.c:(.text+0xa48): undefined reference to `crypto_shash_finup' > > >> ld: ks_hostif.c:(.text+0xa6d): undefined reference to `crypto_destroy_tfm' > > >> > > >> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> > > >> --- > > >> drivers/staging/ks7010/Kconfig | 3 +++ > > >> 1 file changed, 3 insertions(+) > > >> > > >> diff --git a/drivers/staging/ks7010/Kconfig b/drivers/staging/ks7010/Kconfig > > >> index 0987fdc2f70db..8ea6c09286798 100644 > > >> --- a/drivers/staging/ks7010/Kconfig > > >> +++ b/drivers/staging/ks7010/Kconfig > > >> @@ -5,6 +5,9 @@ config KS7010 > > >> select WIRELESS_EXT > > >> select WEXT_PRIV > > >> select FW_LOADER > > >> + select CRYPTO > > >> + select CRYPTO_HASH > > >> + select CRYPTO_MICHAEL_MIC > > > > > > Let's try to rely on 'depend' and not 'select' please. > > > > I used 'select' because it seemed to be the established pattern for > > these options. > > Yes, this is the correct way to do it here. In general, using "depends on" > is better than "select", especially when crossing subsystem boundaries, > however mixing the two is what really hurts because of the circular > dependencies you'd get. > > The only way we could use 'depends on' here would be to change all > the other drivers to do the same. > > > Compare: > > > > $ find -name '*Kconfig*' | xargs git grep 'depends on CRYPTO$' | wc --lines > > 1 > > > > $ find -name '*Kconfig*' | xargs git grep 'select CRYPTO$' | wc --lines > > 66 > > > > $ find -name '*Kconfig*' | xargs git grep 'depends on CRYPTO' | wc --lines > > 87 > > > > $ find -name '*Kconfig*' | xargs git grep 'select CRYPTO' | wc --lines > > 1005 > > > > That said, I have found several other cases where CRYPTO_* algorithms > > are getting 'select'-ed without also selecting CRYPTO/CRYPTO_HASH, so I > > definitely see the problem you're trying to address. > > > > I've added some more people on Cc to see if there is a consensus on the > > best way to do this for the CRYPTO* options going forwards. Thoughts, > > anybody? > > I don't think there is much point in trying to change the > existing pattern for crypto. We might want to change some of those > that use 'depends on' at the moment for consistency, though most of > those look correct as well. Ok, I'll take this as-is then. thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-10-13 13:11 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-11 15:29 [PATCH] staging: ks7010: select CRYPTO_HASH/CRYPTO_MICHAEL_MIC Vegard Nossum 2021-10-11 15:37 ` Greg Kroah-Hartman 2021-10-11 16:55 ` Vegard Nossum 2021-10-13 12:26 ` Greg Kroah-Hartman 2021-10-13 12:51 ` Vegard Nossum 2021-10-13 13:03 ` Arnd Bergmann 2021-10-13 13:11 ` Greg Kroah-Hartman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.