[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20090929101725.a225d382.randy.dunlap@oracle.com>
Date: Tue, 29 Sep 2009 10:17:25 -0700
From: Randy Dunlap <randy.dunlap@...cle.com>
To: Tyler Hicks <tyhicks@...ux.vnet.ibm.com>
Cc: akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
ecryptfs-devel@...ts.launchpad.net,
Dave Hansen <dave@...ux.vnet.ibm.com>
Subject: Re: [PATCH -mmotm] ecryptfs: depends on CRYPTO
On Tue, 29 Sep 2009 12:08:55 -0500 Tyler Hicks wrote:
> On 09/28/2009 07:20 PM, Randy Dunlap wrote:
> > On Mon, 28 Sep 2009 19:10:00 -0500 Tyler Hicks wrote:
> >
> >> On 09/28/2009 03:34 PM, Randy Dunlap wrote:
> >>> From: Randy Dunlap <randy.dunlap@...cle.com>
> >>>
> >>> ecryptfs uses crypto APIs so it should depend on CRYPTO.
> >>> Otherwise many build errors occur. [63 lines not pasted]
> >>>
> >>> Signed-off-by: Randy Dunlap <randy.dunlap@...cle.com>
> >>> ---
> >>> fs/ecryptfs/Kconfig | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> --- mmotm-2009-0925-1435.orig/fs/ecryptfs/Kconfig
> >>> +++ mmotm-2009-0925-1435/fs/ecryptfs/Kconfig
> >>> @@ -1,6 +1,6 @@
> >>> config ECRYPT_FS
> >>> tristate "eCrypt filesystem layer support (EXPERIMENTAL)"
> >>> - depends on EXPERIMENTAL && KEYS && NET
> >>> + depends on EXPERIMENTAL && KEYS && NET && CRYPTO
> >>> select CRYPTO_ECB
> >>> select CRYPTO_CBC
> >>> help
> >>
> >> Hi Randy - Thanks for the patch! Unfortunately, I think it defeats what
> >> Dave Hansen was wanting to do with commit
> >> 382684984e93039a3bbd83b04d341b0ceb831519.
> >>
> >> When I pulled that patch in, I was under the assumption that the select
> >> would also select all necessary dependencies. According to
> >> Documentation/kbuild/kconfig-language.txt, that's not the case:
> >>
> >> select should be used with care. select will force
> >> a symbol to a value without visiting the dependencies.
> >> By abusing select you are able to select a symbol FOO even
> >> if FOO depends on BAR that is not set.
> >>
> >> Maybe we should do it how other folks are tackling this problem and
> >> select CRYPTO, along with CRYPTO_ECB and CRYPTO_CBC. While we're at it,
> >> we should probably throw in CRYPTO_AES (aes-128 is the default cipher,
> >> but the cipher is configurable at mount so it might be too obtrusive for
> >> us to select it) and CRYPTO_MD5 (our default hash alg, not currently
> >> configurable). Also, we don't depend on NET anymore because our netlink
> >> interface is no longer around. It may not hurt to select KEYS, rather
> >> than depend on it. Does all of this sound sane to you?
> >
> > It selects too much stuff. "select" should not be used to enable
> > a full subsystem (that's my general rule, not in kconfig-language.txt).
> > What kconfig-language.txt says that applies here is just after your
> > quoted text:
> >
> > In general use select only for non-visible symbols
> > (no prompts anywhere) and for symbols with no dependencies.
> > That will limit the usefulness but on the other hand avoid
> > the illegal configurations all over.
> >
> > CRYPTO does not fit that.
> >
> > One of the big problems with selecting kconfig symbols (like subsystem
> > ones) is that it makes it difficult to disable that symbol, which some
> > of us often want to do.
> >
> >
> > ---
> > ~Randy
>
> eCryptfs wouldn't be the first to select CRYPTO:
>
> $ grep -r "select CRYPTO$" --include=Kconfig . | wc -l
> 26
Yes, I realize that. and INPUT is selected to much also. :(
> But after trying to deselect CRYPTO with one of my custom configs, I
> realized that you are right. :) Depending on CRYPTO and then selecting
> the proper CRYPTO_* symbols is the way to go.
>
> Applied to
> git://git.kernel.org/pub/scm/linux/kernel/git/ecryptfs/ecryptfs-2.6.git#next
>
> Thanks again!
Thanks, glad that you tried the experiment.
---
~Randy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists