linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* df486a25900 broke arm/mx1_defconfig and probably more
@ 2010-08-25  8:49 Uwe Kleine-König
  2010-08-25  9:05 ` [REGRESSION PATCH] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO Uwe Kleine-König
  0 siblings, 1 reply; 18+ messages in thread
From: Uwe Kleine-König @ 2010-08-25  8:49 UTC (permalink / raw)
  To: Randy Dunlap, Trond Myklebust; +Cc: linux-kernel, J. Bruce Fields

Hello,

trying to compile mx1_defconfig ends in:

	  LD      .tmp_vmlinux1
	fs/built-in.o: In function `nfs_callback_authenticate':
	compr_zlib.c:(.text+0x7d044): undefined reference to `svc_gss_principal'
	make[2]: *** [.tmp_vmlinux1] Error 1
	make[1]: *** [sub-make] Error 2
	make: *** [all] Error 2
	make: Leaving directory `/ptx/work/octopus/WORK_2_A/ukl/kbuild/imxdefconfigs/mx1'

bisecting yields

	df486a2 (NFS: Fix the selection of security flavours in Kconfig)

as first bad commit.

	$ grep -E -e '(NFS|SUNRPC_GSS|CRYPTO|RPCSEC)' .config
	# CONFIG_USB_FUNCTIONFS is not set
	CONFIG_NFS_FS=y
	CONFIG_NFS_V3=y
	# CONFIG_NFS_V3_ACL is not set
	CONFIG_NFS_V4=y
	# CONFIG_NFS_V4_1 is not set
	CONFIG_ROOT_NFS=y
	# CONFIG_NFSD is not set
	CONFIG_NFS_COMMON=y
	# CONFIG_RPCSEC_GSS_SPKM3 is not set
	# CONFIG_CRYPTO is not set

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [REGRESSION PATCH] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO
  2010-08-25  8:49 df486a25900 broke arm/mx1_defconfig and probably more Uwe Kleine-König
@ 2010-08-25  9:05 ` Uwe Kleine-König
  2010-08-27  6:11   ` Uwe Kleine-König
  2010-09-09 16:57   ` [REGRESSION PATCH] " Trond Myklebust
  0 siblings, 2 replies; 18+ messages in thread
From: Uwe Kleine-König @ 2010-08-25  9:05 UTC (permalink / raw)
  To: Randy Dunlap, Trond Myklebust, Linus Torvalds
  Cc: linux-kernel, J. Bruce Fields

This is a follow up to

	df486a2 (NFS: Fix the selection of security flavours in Kconfig)

which broke (among others) arm/mx1_defconfig.

Moreover let NFS_V4 select RPCSEC_GSS_KRB5 again as it was before
df486a2.  This make the dependency more explicit than relying on the no
prompt + default y if !(NFS_V4 || NFSD_V4).

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 fs/nfs/Kconfig     |    2 ++
 fs/nfsd/Kconfig    |    2 ++
 net/sunrpc/Kconfig |    2 +-
 3 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
index 6c2aad4..5b9f870 100644
--- a/fs/nfs/Kconfig
+++ b/fs/nfs/Kconfig
@@ -63,6 +63,8 @@ config NFS_V3_ACL
 config NFS_V4
 	bool "NFS client support for NFS version 4"
 	depends on NFS_FS
+	select CRYPTO # recursive select: RPCSEC_GSS_KRB5 depends on CRYPTO
+	select RPCSEC_GSS_KRB5
 	help
 	  This option enables support for version 4 of the NFS protocol
 	  (RFC 3530) in the kernel's NFS client.
diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
index 95932f5..3678a16 100644
--- a/fs/nfsd/Kconfig
+++ b/fs/nfsd/Kconfig
@@ -69,6 +69,8 @@ config NFSD_V4
 	depends on NFSD && PROC_FS && EXPERIMENTAL
 	select NFSD_V3
 	select FS_POSIX_ACL
+	select CRYPTO # recursive select: RPCSEC_GSS_KRB5 depends on CRYPTO
+	select RPCSEC_GSS_KRB5
 	help
 	  This option enables support in your system's NFS server for
 	  version 4 of the NFS protocol (RFC 3530).
diff --git a/net/sunrpc/Kconfig b/net/sunrpc/Kconfig
index 3376d76..6b661e3 100644
--- a/net/sunrpc/Kconfig
+++ b/net/sunrpc/Kconfig
@@ -20,7 +20,7 @@ config SUNRPC_XPRT_RDMA
 config RPCSEC_GSS_KRB5
 	tristate
 	depends on SUNRPC && CRYPTO
-	prompt "Secure RPC: Kerberos V mechanism" if !(NFS_V4 || NFSD_V4)
+	prompt "Secure RPC: Kerberos V mechanism"
 	default y
 	select SUNRPC_GSS
 	select CRYPTO_MD5
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [REGRESSION PATCH] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO
  2010-08-25  9:05 ` [REGRESSION PATCH] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO Uwe Kleine-König
@ 2010-08-27  6:11   ` Uwe Kleine-König
  2010-08-30  8:26     ` Uwe Kleine-König
  2010-09-09 16:57   ` [REGRESSION PATCH] " Trond Myklebust
  1 sibling, 1 reply; 18+ messages in thread
From: Uwe Kleine-König @ 2010-08-27  6:11 UTC (permalink / raw)
  To: Randy Dunlap, Trond Myklebust, Linus Torvalds, akpm
  Cc: linux-kernel, J. Bruce Fields

On Wed, Aug 25, 2010 at 11:05:19AM +0200, Uwe Kleine-König wrote:
> This is a follow up to
> 
> 	df486a2 (NFS: Fix the selection of security flavours in Kconfig)
> 
> which broke (among others) arm/mx1_defconfig.
> 
> Moreover let NFS_V4 select RPCSEC_GSS_KRB5 again as it was before
> df486a2.  This make the dependency more explicit than relying on the no
> prompt + default y if !(NFS_V4 || NFSD_V4).
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
ping

> ---
>  fs/nfs/Kconfig     |    2 ++
>  fs/nfsd/Kconfig    |    2 ++
>  net/sunrpc/Kconfig |    2 +-
>  3 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
> index 6c2aad4..5b9f870 100644
> --- a/fs/nfs/Kconfig
> +++ b/fs/nfs/Kconfig
> @@ -63,6 +63,8 @@ config NFS_V3_ACL
>  config NFS_V4
>  	bool "NFS client support for NFS version 4"
>  	depends on NFS_FS
> +	select CRYPTO # recursive select: RPCSEC_GSS_KRB5 depends on CRYPTO
> +	select RPCSEC_GSS_KRB5
>  	help
>  	  This option enables support for version 4 of the NFS protocol
>  	  (RFC 3530) in the kernel's NFS client.
> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
> index 95932f5..3678a16 100644
> --- a/fs/nfsd/Kconfig
> +++ b/fs/nfsd/Kconfig
> @@ -69,6 +69,8 @@ config NFSD_V4
>  	depends on NFSD && PROC_FS && EXPERIMENTAL
>  	select NFSD_V3
>  	select FS_POSIX_ACL
> +	select CRYPTO # recursive select: RPCSEC_GSS_KRB5 depends on CRYPTO
> +	select RPCSEC_GSS_KRB5
>  	help
>  	  This option enables support in your system's NFS server for
>  	  version 4 of the NFS protocol (RFC 3530).
> diff --git a/net/sunrpc/Kconfig b/net/sunrpc/Kconfig
> index 3376d76..6b661e3 100644
> --- a/net/sunrpc/Kconfig
> +++ b/net/sunrpc/Kconfig
> @@ -20,7 +20,7 @@ config SUNRPC_XPRT_RDMA
>  config RPCSEC_GSS_KRB5
>  	tristate
>  	depends on SUNRPC && CRYPTO
> -	prompt "Secure RPC: Kerberos V mechanism" if !(NFS_V4 || NFSD_V4)
> +	prompt "Secure RPC: Kerberos V mechanism"
>  	default y
>  	select SUNRPC_GSS
>  	select CRYPTO_MD5
> -- 
> 1.7.1
> 
> 

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [REGRESSION PATCH] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO
  2010-08-27  6:11   ` Uwe Kleine-König
@ 2010-08-30  8:26     ` Uwe Kleine-König
  2010-08-30 10:36       ` Neil Brown
  0 siblings, 1 reply; 18+ messages in thread
From: Uwe Kleine-König @ 2010-08-30  8:26 UTC (permalink / raw)
  To: Randy Dunlap, Trond Myklebust, Linus Torvalds, akpm
  Cc: linux-kernel, J. Bruce Fields, Neil Brown, linux-nfs

[extending Cc: to contain Neil and linux-nfs]

On Fri, Aug 27, 2010 at 08:11:39AM +0200, Uwe Kleine-König wrote:
> On Wed, Aug 25, 2010 at 11:05:19AM +0200, Uwe Kleine-König wrote:
> > This is a follow up to
> > 
> > 	df486a2 (NFS: Fix the selection of security flavours in Kconfig)
> > 
> > which broke (among others) arm/mx1_defconfig.
> > 
> > Moreover let NFS_V4 select RPCSEC_GSS_KRB5 again as it was before
> > df486a2.  This make the dependency more explicit than relying on the no
> > prompt + default y if !(NFS_V4 || NFSD_V4).
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ping
ping^2
> 
> > ---
> >  fs/nfs/Kconfig     |    2 ++
> >  fs/nfsd/Kconfig    |    2 ++
> >  net/sunrpc/Kconfig |    2 +-
> >  3 files changed, 5 insertions(+), 1 deletions(-)
> > 
> > diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
> > index 6c2aad4..5b9f870 100644
> > --- a/fs/nfs/Kconfig
> > +++ b/fs/nfs/Kconfig
> > @@ -63,6 +63,8 @@ config NFS_V3_ACL
> >  config NFS_V4
> >  	bool "NFS client support for NFS version 4"
> >  	depends on NFS_FS
> > +	select CRYPTO # recursive select: RPCSEC_GSS_KRB5 depends on CRYPTO
> > +	select RPCSEC_GSS_KRB5
> >  	help
> >  	  This option enables support for version 4 of the NFS protocol
> >  	  (RFC 3530) in the kernel's NFS client.
> > diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
> > index 95932f5..3678a16 100644
> > --- a/fs/nfsd/Kconfig
> > +++ b/fs/nfsd/Kconfig
> > @@ -69,6 +69,8 @@ config NFSD_V4
> >  	depends on NFSD && PROC_FS && EXPERIMENTAL
> >  	select NFSD_V3
> >  	select FS_POSIX_ACL
> > +	select CRYPTO # recursive select: RPCSEC_GSS_KRB5 depends on CRYPTO
> > +	select RPCSEC_GSS_KRB5
> >  	help
> >  	  This option enables support in your system's NFS server for
> >  	  version 4 of the NFS protocol (RFC 3530).
> > diff --git a/net/sunrpc/Kconfig b/net/sunrpc/Kconfig
> > index 3376d76..6b661e3 100644
> > --- a/net/sunrpc/Kconfig
> > +++ b/net/sunrpc/Kconfig
> > @@ -20,7 +20,7 @@ config SUNRPC_XPRT_RDMA
> >  config RPCSEC_GSS_KRB5
> >  	tristate
> >  	depends on SUNRPC && CRYPTO
> > -	prompt "Secure RPC: Kerberos V mechanism" if !(NFS_V4 || NFSD_V4)
> > +	prompt "Secure RPC: Kerberos V mechanism"
> >  	default y
> >  	select SUNRPC_GSS
> >  	select CRYPTO_MD5
> > -- 
> > 1.7.1

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [REGRESSION PATCH] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO
  2010-08-30  8:26     ` Uwe Kleine-König
@ 2010-08-30 10:36       ` Neil Brown
  2010-08-30 12:10         ` Uwe Kleine-König
  0 siblings, 1 reply; 18+ messages in thread
From: Neil Brown @ 2010-08-30 10:36 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Randy Dunlap, Trond Myklebust, Linus Torvalds, akpm,
	linux-kernel, J. Bruce Fields, linux-nfs

On Mon, 30 Aug 2010 10:26:18 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> [extending Cc: to contain Neil and linux-nfs]
> 
> On Fri, Aug 27, 2010 at 08:11:39AM +0200, Uwe Kleine-König wrote:
> > On Wed, Aug 25, 2010 at 11:05:19AM +0200, Uwe Kleine-König wrote:

I would tend to wait more than 2 days between pings..


> > > This is a follow up to
> > > 
> > > 	df486a2 (NFS: Fix the selection of security flavours in Kconfig)
> > > 
> > > which broke (among others) arm/mx1_defconfig.
> > > 
> > > Moreover let NFS_V4 select RPCSEC_GSS_KRB5 again as it was before
> > > df486a2.  This make the dependency more explicit than relying on the no
> > > prompt + default y if !(NFS_V4 || NFSD_V4).

Maybe if you said a little bit about how it broke?
And I'm not sure of the point of the "recursive dependency" comment below...

I don't fully understand all the issues behind choosing between 'depends' and
'select' (why isn't is 'selects' I wonder - that would be more consistent...)

But that patch seems to make sense to me.

NeilBrown


> > > 
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ping
> ping^2
> > 
> > > ---
> > >  fs/nfs/Kconfig     |    2 ++
> > >  fs/nfsd/Kconfig    |    2 ++
> > >  net/sunrpc/Kconfig |    2 +-
> > >  3 files changed, 5 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
> > > index 6c2aad4..5b9f870 100644
> > > --- a/fs/nfs/Kconfig
> > > +++ b/fs/nfs/Kconfig
> > > @@ -63,6 +63,8 @@ config NFS_V3_ACL
> > >  config NFS_V4
> > >  	bool "NFS client support for NFS version 4"
> > >  	depends on NFS_FS
> > > +	select CRYPTO # recursive select: RPCSEC_GSS_KRB5 depends on CRYPTO
> > > +	select RPCSEC_GSS_KRB5
> > >  	help
> > >  	  This option enables support for version 4 of the NFS protocol
> > >  	  (RFC 3530) in the kernel's NFS client.
> > > diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
> > > index 95932f5..3678a16 100644
> > > --- a/fs/nfsd/Kconfig
> > > +++ b/fs/nfsd/Kconfig
> > > @@ -69,6 +69,8 @@ config NFSD_V4
> > >  	depends on NFSD && PROC_FS && EXPERIMENTAL
> > >  	select NFSD_V3
> > >  	select FS_POSIX_ACL
> > > +	select CRYPTO # recursive select: RPCSEC_GSS_KRB5 depends on CRYPTO
> > > +	select RPCSEC_GSS_KRB5
> > >  	help
> > >  	  This option enables support in your system's NFS server for
> > >  	  version 4 of the NFS protocol (RFC 3530).
> > > diff --git a/net/sunrpc/Kconfig b/net/sunrpc/Kconfig
> > > index 3376d76..6b661e3 100644
> > > --- a/net/sunrpc/Kconfig
> > > +++ b/net/sunrpc/Kconfig
> > > @@ -20,7 +20,7 @@ config SUNRPC_XPRT_RDMA
> > >  config RPCSEC_GSS_KRB5
> > >  	tristate
> > >  	depends on SUNRPC && CRYPTO
> > > -	prompt "Secure RPC: Kerberos V mechanism" if !(NFS_V4 || NFSD_V4)
> > > +	prompt "Secure RPC: Kerberos V mechanism"
> > >  	default y
> > >  	select SUNRPC_GSS
> > >  	select CRYPTO_MD5
> > > -- 
> > > 1.7.1
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [REGRESSION PATCH] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO
  2010-08-30 10:36       ` Neil Brown
@ 2010-08-30 12:10         ` Uwe Kleine-König
  2010-08-30 12:33           ` Uwe Kleine-König
                             ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Uwe Kleine-König @ 2010-08-30 12:10 UTC (permalink / raw)
  To: Neil Brown
  Cc: Randy Dunlap, Trond Myklebust, Linus Torvalds, akpm,
	linux-kernel, J. Bruce Fields, linux-nfs

On Mon, Aug 30, 2010 at 08:36:59PM +1000, Neil Brown wrote:
> On Mon, 30 Aug 2010 10:26:18 +0200
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> 
> > [extending Cc: to contain Neil and linux-nfs]
> > 
> > On Fri, Aug 27, 2010 at 08:11:39AM +0200, Uwe Kleine-König wrote:
> > > On Wed, Aug 25, 2010 at 11:05:19AM +0200, Uwe Kleine-König wrote:
> 
> I would tend to wait more than 2 days between pings..

	ukl@octopus:~/gsrc/linux-2.6$ git rev-parse linus/master
	2bfc96a127bc1cc94d26bfaa40159966064f9c8c
	ukl@octopus:~/gsrc/linux-2.6$ git grep -E CRYPTO= linus/master arch/arm/configs/ | wc -l
	6
	ukl@octopus:~/gsrc/linux-2.6$ git grep -E NFSD?_V4 linus/master arch/arm/configs/ | wc -l
	37

So I think that at least 31 arm-defconfigs don't build because of this
issue.  And as this kind of error greatly hurts automatic bisection I
thought this to be critical enough to be a bit impatient.

> > > > This is a follow up to
> > > > 
> > > > 	df486a2 (NFS: Fix the selection of security flavours in Kconfig)
> > > > 
> > > > which broke (among others) arm/mx1_defconfig.
> > > > 
> > > > Moreover let NFS_V4 select RPCSEC_GSS_KRB5 again as it was before
> > > > df486a2.  This make the dependency more explicit than relying on the no
> > > > prompt + default y if !(NFS_V4 || NFSD_V4).
> 
> Maybe if you said a little bit about how it broke?
  LD      .tmp_vmlinux1
fs/built-in.o: In function `nfs_callback_authenticate':
compr_zlib.c:(.text+0x7c040): undefined reference to `svc_gss_principal'
make[2]: *** [.tmp_vmlinux1] Error 1
make[1]: *** [sub-make] Error 2
make: *** [all] Error 2

I can add this to the commit log.

> And I'm not sure of the point of the "recursive dependency" comment below...
I added this because if kconfig were a bit smarter it would select
CRYPTO, too, if asked to select RPCSEC_GSS_KRB5.  On the
linux-arm-kernel ML Catalin Marinas already thought about making kconfig
smarter and so I wanted to mark the symbol.
 
> I don't fully understand all the issues behind choosing between 'depends' and
> 'select' (why isn't is 'selects' I wonder - that would be more consistent...)
I think it's an imperative, not a normal present tense?!  And note this
is different.  Here it's not depend vs. select but select vs.

	config SOMESYMBOL
		prompt "sometext" if !(NFS_V4 || NFSD_V4)
		default y

So a dependency for NFS_V4 is hidden in net/sunrpc/Kconfig.

> But that patch seems to make sense to me.
Thanks, that's an Ack?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [REGRESSION PATCH] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO
  2010-08-30 12:10         ` Uwe Kleine-König
@ 2010-08-30 12:33           ` Uwe Kleine-König
  2010-08-30 13:50           ` Trond Myklebust
  2010-09-01  8:52           ` [REGRESSION PATCH v2] " Uwe Kleine-König
  2 siblings, 0 replies; 18+ messages in thread
From: Uwe Kleine-König @ 2010-08-30 12:33 UTC (permalink / raw)
  To: Neil Brown
  Cc: Randy Dunlap, Trond Myklebust, Linus Torvalds, akpm,
	linux-kernel, J. Bruce Fields, linux-nfs

On Mon, Aug 30, 2010 at 02:10:22PM +0200, Uwe Kleine-König wrote:
> On Mon, Aug 30, 2010 at 08:36:59PM +1000, Neil Brown wrote:
> > On Mon, 30 Aug 2010 10:26:18 +0200
> > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > 
> > > [extending Cc: to contain Neil and linux-nfs]
> > > 
> > > On Fri, Aug 27, 2010 at 08:11:39AM +0200, Uwe Kleine-König wrote:
> > > > On Wed, Aug 25, 2010 at 11:05:19AM +0200, Uwe Kleine-König wrote:
> > 
> > I would tend to wait more than 2 days between pings..
> 
> 	ukl@octopus:~/gsrc/linux-2.6$ git rev-parse linus/master
> 	2bfc96a127bc1cc94d26bfaa40159966064f9c8c
> 	ukl@octopus:~/gsrc/linux-2.6$ git grep -E CRYPTO= linus/master arch/arm/configs/ | wc -l
> 	6
> 	ukl@octopus:~/gsrc/linux-2.6$ git grep -E NFSD?_V4 linus/master arch/arm/configs/ | wc -l
> 	37
This is wrong, because the last line counts configs having both NFSD_V4
and NFS_V4 twice.  There are "only" 31 that have at least one of NFSD_V4
and NFS_V4.  But only one of these (at572d940hfek_defconfig) has CRYPTO
set, too.
 
> So I think that at least 31 arm-defconfigs don't build because of this
> issue.  And as this kind of error greatly hurts automatic bisection I
> thought this to be critical enough to be a bit impatient.
To extend the test to non-ARM land:

ukl@octopus:~/gsrc/linux-2.6$ git ls-tree -r --name-only linus/master | grep defconfig | xargs grep -l -E 'CONFIG_NFSD?_V4=' | xargs grep -L CONFIG_CRYPTO= | wc -l
108

(Thinking about it again probably not all of them are broken, because
there might be other symbols selecting CRYPTO.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [REGRESSION PATCH] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO
  2010-08-30 12:10         ` Uwe Kleine-König
  2010-08-30 12:33           ` Uwe Kleine-König
@ 2010-08-30 13:50           ` Trond Myklebust
  2010-08-30 14:36             ` Uwe Kleine-König
  2010-09-01  8:52           ` [REGRESSION PATCH v2] " Uwe Kleine-König
  2 siblings, 1 reply; 18+ messages in thread
From: Trond Myklebust @ 2010-08-30 13:50 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Neil Brown, Randy Dunlap, Linus Torvalds, akpm, linux-kernel,
	J. Bruce Fields, linux-nfs

On Mon, 2010-08-30 at 14:10 +0200, Uwe Kleine-König wrote:
> On Mon, Aug 30, 2010 at 08:36:59PM +1000, Neil Brown wrote:
> > On Mon, 30 Aug 2010 10:26:18 +0200
> > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > 
> > > [extending Cc: to contain Neil and linux-nfs]
> > > 
> > > On Fri, Aug 27, 2010 at 08:11:39AM +0200, Uwe Kleine-König wrote:
> > > > On Wed, Aug 25, 2010 at 11:05:19AM +0200, Uwe Kleine-König wrote:
> > 
> > I would tend to wait more than 2 days between pings..
>
> 	ukl@octopus:~/gsrc/linux-2.6$ git rev-parse linus/master
> 	2bfc96a127bc1cc94d26bfaa40159966064f9c8c
> 	ukl@octopus:~/gsrc/linux-2.6$ git grep -E CRYPTO= linus/master arch/arm/configs/ | wc -l
> 	6
> 	ukl@octopus:~/gsrc/linux-2.6$ git grep -E NFSD?_V4 linus/master arch/arm/configs/ | wc -l
> 	37
> 
> So I think that at least 31 arm-defconfigs don't build because of this
> issue.  And as this kind of error greatly hurts automatic bisection I
> thought this to be critical enough to be a bit impatient.

So, why aren't you first and foremost fixing the damned arm-defconfigs?
They are clearly broken if they are auto-selecting NFSv4 without CRYPTO
and RPCSEC_GSS.

> > > > > This is a follow up to
> > > > > 
> > > > > 	df486a2 (NFS: Fix the selection of security flavours in Kconfig)
> > > > > 
> > > > > which broke (among others) arm/mx1_defconfig.
> > > > > 
> > > > > Moreover let NFS_V4 select RPCSEC_GSS_KRB5 again as it was before
> > > > > df486a2.  This make the dependency more explicit than relying on the no
> > > > > prompt + default y if !(NFS_V4 || NFSD_V4).
> > 
> > Maybe if you said a little bit about how it broke?
>   LD      .tmp_vmlinux1
> fs/built-in.o: In function `nfs_callback_authenticate':
> compr_zlib.c:(.text+0x7c040): undefined reference to `svc_gss_principal'
> make[2]: *** [.tmp_vmlinux1] Error 1
> make[1]: *** [sub-make] Error 2
> make: *** [all] Error 2
> 
> I can add this to the commit log.

This is exactly the problem that Randy was seeing _before_ commit
df486a2, so just reverting that patch by adding the selects back into
NFSv4 is wrong.

The right thing to do here (aside from fixing the crummy defconfigs) is
rather to fix nfs_callback_authenticate() to stop depending on GSS
private interfaces such as svc_gss_principal().

> > And I'm not sure of the point of the "recursive dependency" comment below...
> I added this because if kconfig were a bit smarter it would select
> CRYPTO, too, if asked to select RPCSEC_GSS_KRB5.  On the
> linux-arm-kernel ML Catalin Marinas already thought about making kconfig
> smarter and so I wanted to mark the symbol.
>  
> > I don't fully understand all the issues behind choosing between 'depends' and
> > 'select' (why isn't is 'selects' I wonder - that would be more consistent...)
> I think it's an imperative, not a normal present tense?!  And note this
> is different.  Here it's not depend vs. select but select vs.
> 
> 	config SOMESYMBOL
> 		prompt "sometext" if !(NFS_V4 || NFSD_V4)
> 		default y
> 
> So a dependency for NFS_V4 is hidden in net/sunrpc/Kconfig.

You are simply not supposed to be given the option of turning it off if
NFSv4 is selected.

Trond

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [REGRESSION PATCH] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO
  2010-08-30 13:50           ` Trond Myklebust
@ 2010-08-30 14:36             ` Uwe Kleine-König
  0 siblings, 0 replies; 18+ messages in thread
From: Uwe Kleine-König @ 2010-08-30 14:36 UTC (permalink / raw)
  To: Trond Myklebust
  Cc: Neil Brown, Randy Dunlap, Linus Torvalds, akpm, linux-kernel,
	J. Bruce Fields, linux-nfs

Hello Trond,

On Mon, Aug 30, 2010 at 09:50:24AM -0400, Trond Myklebust wrote:
> On Mon, 2010-08-30 at 14:10 +0200, Uwe Kleine-König wrote:
> > On Mon, Aug 30, 2010 at 08:36:59PM +1000, Neil Brown wrote:
> > > On Mon, 30 Aug 2010 10:26:18 +0200
> > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > > 
> > > > [extending Cc: to contain Neil and linux-nfs]
> > > > 
> > > > On Fri, Aug 27, 2010 at 08:11:39AM +0200, Uwe Kleine-König wrote:
> > > > > On Wed, Aug 25, 2010 at 11:05:19AM +0200, Uwe Kleine-König wrote:
> > > 
> > > I would tend to wait more than 2 days between pings..
> >
> > 	ukl@octopus:~/gsrc/linux-2.6$ git rev-parse linus/master
> > 	2bfc96a127bc1cc94d26bfaa40159966064f9c8c
> > 	ukl@octopus:~/gsrc/linux-2.6$ git grep -E CRYPTO= linus/master arch/arm/configs/ | wc -l
> > 	6
> > 	ukl@octopus:~/gsrc/linux-2.6$ git grep -E NFSD?_V4 linus/master arch/arm/configs/ | wc -l
> > 	37
> > 
> > So I think that at least 31 arm-defconfigs don't build because of this
> > issue.  And as this kind of error greatly hurts automatic bisection I
> > thought this to be critical enough to be a bit impatient.
> 
> So, why aren't you first and foremost fixing the damned arm-defconfigs?
They are not broken.  The problem is that it's possible to configure a
kernel that doesn't build.  Note the config resulting from the
mx1_defconfig target fully conform to the restrictions expressed in the
Kconfig files even if arch/arm/configs/mx1_defconfig doesn't.  So if
NFS_V4 was selecting CRYPTO (or CRYPTO would default to y in the
presence of NFS_V4) mx1_defconfig would enable it implicitly.

> They are clearly broken if they are auto-selecting NFSv4 without CRYPTO
> and RPCSEC_GSS.
> 
> > > > > > This is a follow up to
> > > > > > 
> > > > > > 	df486a2 (NFS: Fix the selection of security flavours in Kconfig)
> > > > > > 
> > > > > > which broke (among others) arm/mx1_defconfig.
> > > > > > 
> > > > > > Moreover let NFS_V4 select RPCSEC_GSS_KRB5 again as it was before
> > > > > > df486a2.  This make the dependency more explicit than relying on the no
> > > > > > prompt + default y if !(NFS_V4 || NFSD_V4).
> > > 
> > > Maybe if you said a little bit about how it broke?
> >   LD      .tmp_vmlinux1
> > fs/built-in.o: In function `nfs_callback_authenticate':
> > compr_zlib.c:(.text+0x7c040): undefined reference to `svc_gss_principal'
> > make[2]: *** [.tmp_vmlinux1] Error 1
> > make[1]: *** [sub-make] Error 2
> > make: *** [all] Error 2
> > 
> > I can add this to the commit log.
> 
> This is exactly the problem that Randy was seeing _before_ commit
> df486a2, so just reverting that patch by adding the selects back into
> NFSv4 is wrong.
If NFSD_V4 selects RPCSEC_GSS_KRB5 which in turn selects SUNRPC_GSS the
latter should be enabled in all builds that have NFSD_V4=y (assuming all
dependencies are fulfilled), no?

The problem that needed fixing before your commit was that
RPCSEC_GSS_KRB5 depended on EXPERIMENTAL while NFS_V4 did not (and so
the select RPCSEC_GSS_KRB5 done by NFS_V4 didn't work if EXPERIMENTAL
was unset.)  So the minimal fix would have been to remove the "&&
EXPERIMENTAL" from RPCSEC_GSS_KRB5.

Your commit additionally did the following:

 - change the default of RPCSEC_GSS_KRB5 to y if !(NFS_V4 || NFSD_V4)
 - let RPCSEC_GSS_KRB5 depend on CRYPTO (was *select* CRYPTO before)
 - express the dependency NFSD_V4 -> RPCSEC_GSS_KRB5 at the latter
   symbol (was expressed at NFSD_V4 before)

So because of the second change listed above now my situation is similar
to Randy's earlier, but my problem is I don't have CRYPTO while Randy's
was that he didn't have EXPERIMENTAL.  (That's what I guess, I didn't
read the corresponding thread.)

Subsuming the situation your commit fixed a problem but introduced a
very similar one.

> The right thing to do here (aside from fixing the crummy defconfigs) is
> rather to fix nfs_callback_authenticate() to stop depending on GSS
> private interfaces such as svc_gss_principal().
That would be OK for me, too.  Do you do it?  I guess this has to wait
for the next merge window, so I suggest to still take my patch.

> > > And I'm not sure of the point of the "recursive dependency" comment below...
> > I added this because if kconfig were a bit smarter it would select
> > CRYPTO, too, if asked to select RPCSEC_GSS_KRB5.  On the
> > linux-arm-kernel ML Catalin Marinas already thought about making kconfig
> > smarter and so I wanted to mark the symbol.
> >  
> > > I don't fully understand all the issues behind choosing between 'depends' and
> > > 'select' (why isn't is 'selects' I wonder - that would be more consistent...)
> > I think it's an imperative, not a normal present tense?!  And note this
> > is different.  Here it's not depend vs. select but select vs.
> > 
> > 	config SOMESYMBOL
> > 		prompt "sometext" if !(NFS_V4 || NFSD_V4)
> > 		default y
> > 
> > So a dependency for NFS_V4 is hidden in net/sunrpc/Kconfig.
> 
> You are simply not supposed to be given the option of turning it off if
> NFSv4 is selected.
I understand your construct, but I think it's non sensible to do it this
way.  You're hiding a dependency of NFS_V4 this way (to the developper,
not the user configuring the kernel).

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [REGRESSION PATCH v2] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO
  2010-08-30 12:10         ` Uwe Kleine-König
  2010-08-30 12:33           ` Uwe Kleine-König
  2010-08-30 13:50           ` Trond Myklebust
@ 2010-09-01  8:52           ` Uwe Kleine-König
  2010-09-01 13:17             ` Trond Myklebust
  2 siblings, 1 reply; 18+ messages in thread
From: Uwe Kleine-König @ 2010-09-01  8:52 UTC (permalink / raw)
  To: linux-nfs, linux-kernel
  Cc: akpm, J. Bruce Fields, Trond Myklebust, Randy Dunlap, Neil Brown,
	Linus Torvalds

Hello,

here comes a v2 of the patch that improves the commit log with a more
detailed analysis of the breakage introduced by df486a2
(= v2.6.36-rc2~34^2~1 BTW) and additionally undoes the "default y" for
RPCSEC_GSS_KRB5.

So compared to the state before df486a2 the changes are:

        NFS_V4 selects CRYPTO
        NFSD_V4 selects CRYPTO
	RPCSEC_GSS_KRB5 doesn't depend on EXPERIMENTAL anymore
	RPCSEC_GSS_KRB5 now depends on CRYPTO instead of selecting it

Best regards
Uwe

----------------------------->8----------------------------

This is a follow up to

	df486a2 (NFS: Fix the selection of security flavours in Kconfig)

Before df486a2 NFS_V4 selected RPCSEC_GSS_KRB5 but didn't enforce the
latter's dependency EXPERIMENTAL.  df486a2 removed RPCSEC_GSS_KRB5's
dependency on EXPERIMENTAL but additionally let it depend on CRYPTO
(instead of select CRYPTO before).  So it was still possible to have a
config that has NFS_V4 but not RPCSEC_GSS_KRB5.  Moreover df486a2
changed the dependency of NFS_V4 and NFSD_V4 on RPCSEC_GSS_KRB5 from

	config NFS_V4
		...
		select RPCSEC_GSS_KRB5

to

	config RPCSEC_GSS_KRB5
		...
		prompt "..." if !(NFS_V4 || NFSD_V4)
		default y

This works but is ugly as it hides the dependencies of NFSD?_V4 in a
different Kconfig file.  So this is undone here, too.

The following ARM defconfigs were affected by this problem:
	mx1 pxa3xx qil-a9260 usb-a9260 usb-a9263

These builds ended in:

	  LD      init/built-in.o
	  LD      .tmp_vmlinux1
	fs/built-in.o: In function `nfs_callback_authenticate':
	compr_zlib.c:(.text+0x7c040): undefined reference to `svc_gss_principal'
	make[2]: *** [.tmp_vmlinux1] Error 1
	make[1]: *** [sub-make] Error 2
	make: *** [all] Error 2

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 fs/nfs/Kconfig     |    2 ++
 fs/nfsd/Kconfig    |    2 ++
 net/sunrpc/Kconfig |    6 ++----
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
index 6c2aad4..5b9f870 100644
--- a/fs/nfs/Kconfig
+++ b/fs/nfs/Kconfig
@@ -63,6 +63,8 @@ config NFS_V3_ACL
 config NFS_V4
 	bool "NFS client support for NFS version 4"
 	depends on NFS_FS
+	select CRYPTO # recursive select: RPCSEC_GSS_KRB5 depends on CRYPTO
+	select RPCSEC_GSS_KRB5
 	help
 	  This option enables support for version 4 of the NFS protocol
 	  (RFC 3530) in the kernel's NFS client.
diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
index 95932f5..3678a16 100644
--- a/fs/nfsd/Kconfig
+++ b/fs/nfsd/Kconfig
@@ -69,6 +69,8 @@ config NFSD_V4
 	depends on NFSD && PROC_FS && EXPERIMENTAL
 	select NFSD_V3
 	select FS_POSIX_ACL
+	select CRYPTO # recursive select: RPCSEC_GSS_KRB5 depends on CRYPTO
+	select RPCSEC_GSS_KRB5
 	help
 	  This option enables support in your system's NFS server for
 	  version 4 of the NFS protocol (RFC 3530).
diff --git a/net/sunrpc/Kconfig b/net/sunrpc/Kconfig
index 3376d76..523be24 100644
--- a/net/sunrpc/Kconfig
+++ b/net/sunrpc/Kconfig
@@ -18,10 +18,8 @@ config SUNRPC_XPRT_RDMA
 	  If unsure, say N.
 
 config RPCSEC_GSS_KRB5
-	tristate
+	tristate "Secure RPC: Kerberos V mechanism"
 	depends on SUNRPC && CRYPTO
-	prompt "Secure RPC: Kerberos V mechanism" if !(NFS_V4 || NFSD_V4)
-	default y
 	select SUNRPC_GSS
 	select CRYPTO_MD5
 	select CRYPTO_DES
@@ -35,7 +33,7 @@ config RPCSEC_GSS_KRB5
 	  available from http://linux-nfs.org/.  In addition, user-space
 	  Kerberos support should be installed.
 
-	  If unsure, say Y.
+	  If unsure, say N.
 
 config RPCSEC_GSS_SPKM3
 	tristate "Secure RPC: SPKM3 mechanism (EXPERIMENTAL)"
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [REGRESSION PATCH v2] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO
  2010-09-01  8:52           ` [REGRESSION PATCH v2] " Uwe Kleine-König
@ 2010-09-01 13:17             ` Trond Myklebust
  2010-09-01 13:50               ` Uwe Kleine-König
  0 siblings, 1 reply; 18+ messages in thread
From: Trond Myklebust @ 2010-09-01 13:17 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: linux-nfs, linux-kernel, akpm, J. Bruce Fields, Randy Dunlap,
	Neil Brown, Linus Torvalds

On Wed, 2010-09-01 at 10:52 +0200, Uwe Kleine-König wrote:
> Hello,
> 
> here comes a v2 of the patch that improves the commit log with a more
> detailed analysis of the breakage introduced by df486a2
> (= v2.6.36-rc2~34^2~1 BTW) and additionally undoes the "default y" for
> RPCSEC_GSS_KRB5.
> 
> So compared to the state before df486a2 the changes are:
> 
>         NFS_V4 selects CRYPTO
>         NFSD_V4 selects CRYPTO
> 	RPCSEC_GSS_KRB5 doesn't depend on EXPERIMENTAL anymore
> 	RPCSEC_GSS_KRB5 now depends on CRYPTO instead of selecting it
> 
> Best regards
> Uwe
> 
> ----------------------------->8----------------------------
> 
> This is a follow up to
> 
> 	df486a2 (NFS: Fix the selection of security flavours in Kconfig)
> 
> Before df486a2 NFS_V4 selected RPCSEC_GSS_KRB5 but didn't enforce the
> latter's dependency EXPERIMENTAL.  df486a2 removed RPCSEC_GSS_KRB5's
> dependency on EXPERIMENTAL but additionally let it depend on CRYPTO
> (instead of select CRYPTO before).  So it was still possible to have a
> config that has NFS_V4 but not RPCSEC_GSS_KRB5.  Moreover df486a2
> changed the dependency of NFS_V4 and NFSD_V4 on RPCSEC_GSS_KRB5 from

As I said, the fix is to remove that dependency. I have a fix for the
NFS client, but the server has more insidious dependencies on RPCSEC_GSS
due to a poorly designed SECINFO implementation.

Trond

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [REGRESSION PATCH v2] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO
  2010-09-01 13:17             ` Trond Myklebust
@ 2010-09-01 13:50               ` Uwe Kleine-König
  2010-09-03 20:24                 ` Uwe Kleine-König
  0 siblings, 1 reply; 18+ messages in thread
From: Uwe Kleine-König @ 2010-09-01 13:50 UTC (permalink / raw)
  To: Trond Myklebust
  Cc: linux-nfs, linux-kernel, akpm, J. Bruce Fields, Randy Dunlap,
	Neil Brown, Linus Torvalds

On Wed, Sep 01, 2010 at 09:17:04AM -0400, Trond Myklebust wrote:
> On Wed, 2010-09-01 at 10:52 +0200, Uwe Kleine-König wrote:
> > Hello,
> > 
> > here comes a v2 of the patch that improves the commit log with a more
> > detailed analysis of the breakage introduced by df486a2
> > (= v2.6.36-rc2~34^2~1 BTW) and additionally undoes the "default y" for
> > RPCSEC_GSS_KRB5.
> > 
> > So compared to the state before df486a2 the changes are:
> > 
> >         NFS_V4 selects CRYPTO
> >         NFSD_V4 selects CRYPTO
> > 	RPCSEC_GSS_KRB5 doesn't depend on EXPERIMENTAL anymore
> > 	RPCSEC_GSS_KRB5 now depends on CRYPTO instead of selecting it
> > 
> > Best regards
> > Uwe
> > 
> > ----------------------------->8----------------------------
> > 
> > This is a follow up to
> > 
> > 	df486a2 (NFS: Fix the selection of security flavours in Kconfig)
> > 
> > Before df486a2 NFS_V4 selected RPCSEC_GSS_KRB5 but didn't enforce the
> > latter's dependency EXPERIMENTAL.  df486a2 removed RPCSEC_GSS_KRB5's
> > dependency on EXPERIMENTAL but additionally let it depend on CRYPTO
> > (instead of select CRYPTO before).  So it was still possible to have a
> > config that has NFS_V4 but not RPCSEC_GSS_KRB5.  Moreover df486a2
> > changed the dependency of NFS_V4 and NFSD_V4 on RPCSEC_GSS_KRB5 from
> 
> As I said, the fix is to remove that dependency. I have a fix for the
> NFS client, but the server has more insidious dependencies on RPCSEC_GSS
> due to a poorly designed SECINFO implementation.
Yes, I still remember this, so I suggest to take my patch before 2.6.36
and you can fix it in the merge window for 2.6.37, no?

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [REGRESSION PATCH v2] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO
  2010-09-01 13:50               ` Uwe Kleine-König
@ 2010-09-03 20:24                 ` Uwe Kleine-König
  0 siblings, 0 replies; 18+ messages in thread
From: Uwe Kleine-König @ 2010-09-03 20:24 UTC (permalink / raw)
  To: Trond Myklebust
  Cc: linux-nfs, linux-kernel, akpm, J. Bruce Fields, Randy Dunlap,
	Neil Brown, Linus Torvalds

Hello Trond,

On Wed, Sep 01, 2010 at 03:50:41PM +0200, Uwe Kleine-König wrote:
> On Wed, Sep 01, 2010 at 09:17:04AM -0400, Trond Myklebust wrote:
> > On Wed, 2010-09-01 at 10:52 +0200, Uwe Kleine-König wrote:
> > > Hello,
> > > 
> > > here comes a v2 of the patch that improves the commit log with a more
> > > detailed analysis of the breakage introduced by df486a2
> > > (= v2.6.36-rc2~34^2~1 BTW) and additionally undoes the "default y" for
> > > RPCSEC_GSS_KRB5.
> > > 
> > > So compared to the state before df486a2 the changes are:
> > > 
> > >         NFS_V4 selects CRYPTO
> > >         NFSD_V4 selects CRYPTO
> > > 	RPCSEC_GSS_KRB5 doesn't depend on EXPERIMENTAL anymore
> > > 	RPCSEC_GSS_KRB5 now depends on CRYPTO instead of selecting it
> > > 
> > > Best regards
> > > Uwe
> > > 
> > > ----------------------------->8----------------------------
> > > 
> > > This is a follow up to
> > > 
> > > 	df486a2 (NFS: Fix the selection of security flavours in Kconfig)
> > > 
> > > Before df486a2 NFS_V4 selected RPCSEC_GSS_KRB5 but didn't enforce the
> > > latter's dependency EXPERIMENTAL.  df486a2 removed RPCSEC_GSS_KRB5's
> > > dependency on EXPERIMENTAL but additionally let it depend on CRYPTO
> > > (instead of select CRYPTO before).  So it was still possible to have a
> > > config that has NFS_V4 but not RPCSEC_GSS_KRB5.  Moreover df486a2
> > > changed the dependency of NFS_V4 and NFSD_V4 on RPCSEC_GSS_KRB5 from
> > 
> > As I said, the fix is to remove that dependency. I have a fix for the
> > NFS client, but the server has more insidious dependencies on RPCSEC_GSS
> > due to a poorly designed SECINFO implementation.
> Yes, I still remember this, so I suggest to take my patch before 2.6.36
> and you can fix it in the merge window for 2.6.37, no?
ping

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [REGRESSION PATCH] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO
  2010-08-25  9:05 ` [REGRESSION PATCH] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO Uwe Kleine-König
  2010-08-27  6:11   ` Uwe Kleine-König
@ 2010-09-09 16:57   ` Trond Myklebust
  2010-09-09 18:14     ` Uwe Kleine-König
  1 sibling, 1 reply; 18+ messages in thread
From: Trond Myklebust @ 2010-09-09 16:57 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Randy Dunlap, Linus Torvalds, linux-kernel, J. Bruce Fields

On Wed, 2010-08-25 at 11:05 +0200, Uwe Kleine-König wrote:
> This is a follow up to
> 
> 	df486a2 (NFS: Fix the selection of security flavours in Kconfig)
> 
> which broke (among others) arm/mx1_defconfig.
> 
> Moreover let NFS_V4 select RPCSEC_GSS_KRB5 again as it was before
> df486a2.  This make the dependency more explicit than relying on the no
> prompt + default y if !(NFS_V4 || NFSD_V4).

Hi Uwe,

Having looked more closely at the actual dependencies in the NFSv4
client and server (see the changelog below), I believe the following is
the correct patch. It ensures that the RPCSEC_GSS module is always
selected, and does not introduce any unnecessary dependencies on CRYPTO.

Cheers
  Trond

-----------------------------------------------------------------------
SUNRPC: Fix the NFSv4 and RPCSEC_GSS Kconfig dependencies

From: Trond Myklebust <Trond.Myklebust@netapp.com>

The NFSv4 client's callback server calls svc_gss_principal(), which
is defined in the auth_rpcgss.ko

The NFSv4 server has the same dependency, and in addition calls
svcauth_gss_flavor(), gss_mech_get_by_pseudoflavor(),
gss_pseudoflavor_to_service() and gss_mech_put() from the same module.

The module auth_rpcgss itself has no dependencies aside from sunrpc,
so we only need to select RPCSEC_GSS.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/Kconfig  |    1 +
 fs/nfsd/Kconfig |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)


diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
index 6c2aad4..f7e13db 100644
--- a/fs/nfs/Kconfig
+++ b/fs/nfs/Kconfig
@@ -63,6 +63,7 @@ config NFS_V3_ACL
 config NFS_V4
 	bool "NFS client support for NFS version 4"
 	depends on NFS_FS
+	select SUNRPC_GSS
 	help
 	  This option enables support for version 4 of the NFS protocol
 	  (RFC 3530) in the kernel's NFS client.
diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
index 95932f5..4264377 100644
--- a/fs/nfsd/Kconfig
+++ b/fs/nfsd/Kconfig
@@ -69,6 +69,7 @@ config NFSD_V4
 	depends on NFSD && PROC_FS && EXPERIMENTAL
 	select NFSD_V3
 	select FS_POSIX_ACL
+	select SUNRPC_GSS
 	help
 	  This option enables support in your system's NFS server for
 	  version 4 of the NFS protocol (RFC 3530).


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [REGRESSION PATCH] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO
  2010-09-09 16:57   ` [REGRESSION PATCH] " Trond Myklebust
@ 2010-09-09 18:14     ` Uwe Kleine-König
  2010-09-27 10:41       ` [PATCH] sunrpc: prompt for RPCSEC_GSS_KRB5 even if NFS_V4 is enabled Uwe Kleine-König
  0 siblings, 1 reply; 18+ messages in thread
From: Uwe Kleine-König @ 2010-09-09 18:14 UTC (permalink / raw)
  To: Trond Myklebust
  Cc: Randy Dunlap, Linus Torvalds, linux-kernel, J. Bruce Fields

Hi Trond,

On Thu, Sep 09, 2010 at 12:57:28PM -0400, Trond Myklebust wrote:
> On Wed, 2010-08-25 at 11:05 +0200, Uwe Kleine-König wrote:
> > This is a follow up to
> > 
> > 	df486a2 (NFS: Fix the selection of security flavours in Kconfig)
> > 
> > which broke (among others) arm/mx1_defconfig.
> > 
> > Moreover let NFS_V4 select RPCSEC_GSS_KRB5 again as it was before
> > df486a2.  This make the dependency more explicit than relying on the no
> > prompt + default y if !(NFS_V4 || NFSD_V4).
> Having looked more closely at the actual dependencies in the NFSv4
> client and server (see the changelog below), I believe the following is
> the correct patch. It ensures that the RPCSEC_GSS module is always
> selected, and does not introduce any unnecessary dependencies on CRYPTO.
Fine, works for me.  That's even better than selecting CRYPTO.

Still, wouldn't be the patch below more complete?

Best regards
Uwe

diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
index 6c2aad4..f7e13db 100644
--- a/fs/nfs/Kconfig
+++ b/fs/nfs/Kconfig
@@ -63,6 +63,7 @@ config NFS_V3_ACL
 config NFS_V4
 	bool "NFS client support for NFS version 4"
 	depends on NFS_FS
+	select SUNRPC_GSS
 	help
 	  This option enables support for version 4 of the NFS protocol
 	  (RFC 3530) in the kernel's NFS client.
diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
index 95932f5..4264377 100644
--- a/fs/nfsd/Kconfig
+++ b/fs/nfsd/Kconfig
@@ -69,6 +69,7 @@ config NFSD_V4
 	depends on NFSD && PROC_FS && EXPERIMENTAL
 	select NFSD_V3
 	select FS_POSIX_ACL
+	select SUNRPC_GSS
 	help
 	  This option enables support in your system's NFS server for
 	  version 4 of the NFS protocol (RFC 3530).
diff --git a/net/sunrpc/Kconfig b/net/sunrpc/Kconfig
index 3376d76..442efe1 100644
--- a/net/sunrpc/Kconfig
+++ b/net/sunrpc/Kconfig
@@ -20,8 +20,7 @@ config SUNRPC_XPRT_RDMA
 config RPCSEC_GSS_KRB5
 	tristate
 	depends on SUNRPC && CRYPTO
-	prompt "Secure RPC: Kerberos V mechanism" if !(NFS_V4 || NFSD_V4)
-	default y
+	prompt "Secure RPC: Kerberos V mechanism"
 	select SUNRPC_GSS
 	select CRYPTO_MD5
 	select CRYPTO_DES

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] sunrpc: prompt for RPCSEC_GSS_KRB5 even if NFS_V4 is enabled
  2010-09-09 18:14     ` Uwe Kleine-König
@ 2010-09-27 10:41       ` Uwe Kleine-König
  2010-09-27 11:39         ` Trond Myklebust
  0 siblings, 1 reply; 18+ messages in thread
From: Uwe Kleine-König @ 2010-09-27 10:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: Trond Myklebust, J. Bruce Fields, Neil Brown, David S. Miller,
	Uwe Kleine-König, Randy Dunlap, linux-nfs, netdev

NFS_V4 works fine without RPCSEC_GSS_KRB5 (even without CRYPTO).
This dependency was introduced in

	df486a2 (NFS: Fix the selection of security flavours in Kconfig)

to fix a build failure as RPCSEC_GSS_KRB5 was thought to be needed for
NFS_V4.  The fix didn't work completely as NFS_V4 didn't enforce CRYPTO
and so the select on RPCSEC_GSS_KRB5 didn't work in all situations (e.g.
arm/mx1_defconfig).

This was rectified by

	827e345 (SUNRPC: Fix the NFSv4 and RPCSEC_GSS Kconfig dependencies)

but the magic for RPCSEC_GSS_KRB5 introduced by df486a2 wasn't reverted.

Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

after Trond sent me the patch that later ended in 827e345702 I suggested
to fold the patch below into it[1], but without reaction and success as I
noticed just now. :-(

Best regards
Uwe

[1] http://thread.gmane.org/gmane.linux.kernel/1027380/focus=1033847

 net/sunrpc/Kconfig |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/Kconfig b/net/sunrpc/Kconfig
index 3376d76..442efe1 100644
--- a/net/sunrpc/Kconfig
+++ b/net/sunrpc/Kconfig
@@ -20,8 +20,7 @@ config SUNRPC_XPRT_RDMA
 config RPCSEC_GSS_KRB5
 	tristate
 	depends on SUNRPC && CRYPTO
-	prompt "Secure RPC: Kerberos V mechanism" if !(NFS_V4 || NFSD_V4)
-	default y
+	prompt "Secure RPC: Kerberos V mechanism"
 	select SUNRPC_GSS
 	select CRYPTO_MD5
 	select CRYPTO_DES
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] sunrpc: prompt for RPCSEC_GSS_KRB5 even if NFS_V4 is enabled
  2010-09-27 10:41       ` [PATCH] sunrpc: prompt for RPCSEC_GSS_KRB5 even if NFS_V4 is enabled Uwe Kleine-König
@ 2010-09-27 11:39         ` Trond Myklebust
  2010-09-27 12:54           ` Uwe Kleine-König
  0 siblings, 1 reply; 18+ messages in thread
From: Trond Myklebust @ 2010-09-27 11:39 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: linux-kernel, J. Bruce Fields, Neil Brown, David S. Miller,
	Randy Dunlap, linux-nfs, netdev

On Mon, 2010-09-27 at 12:41 +0200, Uwe Kleine-König wrote:
> NFS_V4 works fine without RPCSEC_GSS_KRB5 (even without CRYPTO).
> This dependency was introduced in
> 
> 	df486a2 (NFS: Fix the selection of security flavours in Kconfig)
> 
> to fix a build failure as RPCSEC_GSS_KRB5 was thought to be needed for
> NFS_V4.  The fix didn't work completely as NFS_V4 didn't enforce CRYPTO
> and so the select on RPCSEC_GSS_KRB5 didn't work in all situations (e.g.
> arm/mx1_defconfig).
> 
> This was rectified by
> 
> 	827e345 (SUNRPC: Fix the NFSv4 and RPCSEC_GSS Kconfig dependencies)
> 
> but the magic for RPCSEC_GSS_KRB5 introduced by df486a2 wasn't reverted.
> 
> Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> after Trond sent me the patch that later ended in 827e345702 I suggested
> to fold the patch below into it[1], but without reaction and success as I
> noticed just now. :-(
> 
> Best regards
> Uwe

That's because you completely fail to justify why should we change the
behaviour to suddenly make RPCSEC_GSS_KRB5 optional for NFSv4. That has
never been the case before.

Trond

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] sunrpc: prompt for RPCSEC_GSS_KRB5 even if NFS_V4 is enabled
  2010-09-27 11:39         ` Trond Myklebust
@ 2010-09-27 12:54           ` Uwe Kleine-König
  0 siblings, 0 replies; 18+ messages in thread
From: Uwe Kleine-König @ 2010-09-27 12:54 UTC (permalink / raw)
  To: Trond Myklebust
  Cc: linux-kernel, J. Bruce Fields, Neil Brown, David S. Miller,
	Randy Dunlap, linux-nfs, netdev

Hi Trond,

On Mon, Sep 27, 2010 at 07:39:32AM -0400, Trond Myklebust wrote:
> On Mon, 2010-09-27 at 12:41 +0200, Uwe Kleine-König wrote:
> > NFS_V4 works fine without RPCSEC_GSS_KRB5 (even without CRYPTO).
> > This dependency was introduced in
> > 
> > 	df486a2 (NFS: Fix the selection of security flavours in Kconfig)
> > 
> > to fix a build failure as RPCSEC_GSS_KRB5 was thought to be needed for
> > NFS_V4.  The fix didn't work completely as NFS_V4 didn't enforce CRYPTO
> > and so the select on RPCSEC_GSS_KRB5 didn't work in all situations (e.g.
> > arm/mx1_defconfig).
> > 
> > This was rectified by
> > 
> > 	827e345 (SUNRPC: Fix the NFSv4 and RPCSEC_GSS Kconfig dependencies)
> > 
> > but the magic for RPCSEC_GSS_KRB5 introduced by df486a2 wasn't reverted.
> > 
> > Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > Hello,
> > 
> > after Trond sent me the patch that later ended in 827e345702 I suggested
> > to fold the patch below into it[1], but without reaction and success as I
> > noticed just now. :-(
> > 
> > Best regards
> > Uwe
> 
> That's because you completely fail to justify why should we change the
> behaviour to suddenly make RPCSEC_GSS_KRB5 optional for NFSv4. That has
> never been the case before.
My intention is not to make "RPCSEC_GSS_KRB5 optional for NFSv4".  First
I saw a build failure and then I wondered if the fix was optimal.  After
reading the log of 827e345 I thought NFSv4 doesn't depend on
RPCSEC_GSS_KRB5, still more considering that 827e345 was your fix after
I suggested to select CRYPTO to enforce RPCSEC_GSS_KRB5 again.

Currently you can have NFSv4 without RPCSEC_GSS_KRB5 because if you
don't have CRYPTO RPCSEC_GSS_KRB5 is off, too, even if it defaults to
yes and there's no prompt.  (Selecting would not work, too.)
And note that RPCSEC_GSS_KRB5 already selects SUNRPC_GSS, so 827e345
doesn't do anything useful if NFS_V4 really needs RPCSEC_GSS_KRB5.

So either we should really enforce RPCSEC_GSS_KRB5 if NFS_V4 is selected
(by letting one of these select CRYPTO, see e.g. my first patch, or by
letting NFS_V4 depend on CRYPTO) or make it optional in all cases (as it
is already now in some cases).

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2010-09-27 12:54 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-25  8:49 df486a25900 broke arm/mx1_defconfig and probably more Uwe Kleine-König
2010-08-25  9:05 ` [REGRESSION PATCH] NFS: let NFS_V4 and NFSD_V4 enforce CRYPTO Uwe Kleine-König
2010-08-27  6:11   ` Uwe Kleine-König
2010-08-30  8:26     ` Uwe Kleine-König
2010-08-30 10:36       ` Neil Brown
2010-08-30 12:10         ` Uwe Kleine-König
2010-08-30 12:33           ` Uwe Kleine-König
2010-08-30 13:50           ` Trond Myklebust
2010-08-30 14:36             ` Uwe Kleine-König
2010-09-01  8:52           ` [REGRESSION PATCH v2] " Uwe Kleine-König
2010-09-01 13:17             ` Trond Myklebust
2010-09-01 13:50               ` Uwe Kleine-König
2010-09-03 20:24                 ` Uwe Kleine-König
2010-09-09 16:57   ` [REGRESSION PATCH] " Trond Myklebust
2010-09-09 18:14     ` Uwe Kleine-König
2010-09-27 10:41       ` [PATCH] sunrpc: prompt for RPCSEC_GSS_KRB5 even if NFS_V4 is enabled Uwe Kleine-König
2010-09-27 11:39         ` Trond Myklebust
2010-09-27 12:54           ` Uwe Kleine-König

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).