linux-ext4 - Re: [PATCH V2 02/13] Consolidate "read callbacks" into a new file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <8267479.irDUbO2F3m@localhost.localdomain>
Date:   Wed, 01 May 2019 18:02:39 +0530
From:   Chandan Rajendra <chandan@...ux.ibm.com>
To:     Eric Biggers <ebiggers@...nel.org>
Cc:     linux-fsdevel@...r.kernel.org, linux-ext4@...r.kernel.org,
        linux-f2fs-devel@...ts.sourceforge.net,
        linux-fscrypt@...r.kernel.org, tytso@....edu,
        adilger.kernel@...ger.ca, jaegeuk@...nel.org, yuchao0@...wei.com,
        hch@...radead.org
Subject: Re: [PATCH V2 02/13] Consolidate "read callbacks" into a new file

On Tuesday, April 30, 2019 11:35:08 PM IST Eric Biggers wrote:
> On Sun, Apr 28, 2019 at 10:01:10AM +0530, Chandan Rajendra wrote:
> > The "read callbacks" code is used by both Ext4 and F2FS. Hence to
> > remove duplicity, this commit moves the code into
> > include/linux/read_callbacks.h and fs/read_callbacks.c.
> > 
> > The corresponding decrypt and verity "work" functions have been moved
> > inside fscrypt and fsverity sources. With these in place, the read
> > callbacks code now has to just invoke enqueue functions provided by
> > fscrypt and fsverity.
> > 
> > Signed-off-by: Chandan Rajendra <chandan@...ux.ibm.com>
> > ---
> >  fs/Kconfig                     |   4 +
> >  fs/Makefile                    |   4 +
> >  fs/crypto/Kconfig              |   1 +
> >  fs/crypto/bio.c                |  23 ++---
> >  fs/crypto/crypto.c             |  17 +--
> >  fs/crypto/fscrypt_private.h    |   3 +
> >  fs/ext4/ext4.h                 |   2 -
> >  fs/ext4/readpage.c             | 183 +++++----------------------------
> >  fs/ext4/super.c                |   9 +-
> >  fs/f2fs/data.c                 | 148 ++++----------------------
> >  fs/f2fs/super.c                |   9 +-
> >  fs/read_callbacks.c            | 136 ++++++++++++++++++++++++
> >  fs/verity/Kconfig              |   1 +
> >  fs/verity/verify.c             |  12 +++
> >  include/linux/fscrypt.h        |  20 +---
> >  include/linux/read_callbacks.h |  21 ++++
> >  16 files changed, 251 insertions(+), 342 deletions(-)
> >  create mode 100644 fs/read_callbacks.c
> >  create mode 100644 include/linux/read_callbacks.h
> > 
> > diff --git a/fs/Kconfig b/fs/Kconfig
> > index 97f9eb8df713..03084f2dbeaf 100644
> > --- a/fs/Kconfig
> > +++ b/fs/Kconfig
> > @@ -308,6 +308,10 @@ config NFS_COMMON
> >  	depends on NFSD || NFS_FS || LOCKD
> >  	default y
> >  
> > +config FS_READ_CALLBACKS
> > +       bool
> > +       default n
> > +
> >  source "net/sunrpc/Kconfig"
> >  source "fs/ceph/Kconfig"
> >  source "fs/cifs/Kconfig"
> 
> This shouldn't be under the 'if NETWORK_FILESYSTEMS' block, since it has nothing
> to do with network filesystems.  When trying to compile this I got:
> 
> 	WARNING: unmet direct dependencies detected for FS_READ_CALLBACKS
> 	  Depends on [n]: NETWORK_FILESYSTEMS [=n]
> 	  Selected by [y]:
> 	  - FS_ENCRYPTION [=y]
> 	  - FS_VERITY [=y]
> 
> Perhaps put it just below FS_IOMAP?
> 
> > diff --git a/fs/Makefile b/fs/Makefile
> > index 9dd2186e74b5..e0c0fce8cf40 100644
> > --- a/fs/Makefile
> > +++ b/fs/Makefile
> > @@ -21,6 +21,10 @@ else
> >  obj-y +=	no-block.o
> >  endif
> >  
> > +ifeq ($(CONFIG_FS_READ_CALLBACKS),y)
> > +obj-y +=	read_callbacks.o
> > +endif
> > +
> >  obj-$(CONFIG_PROC_FS) += proc_namespace.o
> >  
> >  obj-y				+= notify/
> > diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
> > index f0de238000c0..163c328bcbd4 100644
> > --- a/fs/crypto/Kconfig
> > +++ b/fs/crypto/Kconfig
> > @@ -8,6 +8,7 @@ config FS_ENCRYPTION
> >  	select CRYPTO_CTS
> >  	select CRYPTO_SHA256
> >  	select KEYS
> > +	select FS_READ_CALLBACKS
> >  	help
> >  	  Enable encryption of files and directories.  This
> >  	  feature is similar to ecryptfs, but it is more memory
> 
> This selection needs to be conditional on BLOCK.
> 
> 	select FS_READ_CALLBACKS if BLOCK
> 
> Otherwise, building without BLOCK and with UBIFS encryption support fails.
> 
> 	fs/read_callbacks.c: In function ‘end_read_callbacks’:
> 	fs/read_callbacks.c:34:23: error: storage size of ‘iter_all’ isn’t known
> 	  struct bvec_iter_all iter_all;
> 			       ^~~~~~~~
> 	fs/read_callbacks.c:37:20: error: dereferencing pointer to incomplete type ‘struct buffer_head’
> 	   if (!PageError(bh->b_page))
> 
> 	[...]
>

I will fix this in the next version of this patchset.

-- 
chandan