27 Aug, 2010

3 commits

  • Fixes a regression caused by 21edad32205e97dc7ccb81a85234c77e760364c8

    When file name encryption was enabled, ecryptfs_lookup() failed to use
    the encrypted and encoded version of the upper, plaintext, file name
    when performing a lookup in the lower file system. This made it
    impossible to lookup existing encrypted file names and any newly created
    files would have plaintext file names in the lower file system.

    https://bugs.launchpad.net/ecryptfs/+bug/623087

    Signed-off-by: Tyler Hicks

    Tyler Hicks
     
  • Some ecryptfs init functions are not prefixed by __init and thus not
    freed after initialization. This patch saved about 1kB in ecryptfs
    module.

    Signed-off-by: Jerome Marchand
    Signed-off-by: Tyler Hicks

    Jerome Marchand
     
  • In this code, 0 is returned on memory allocation failure, even though other
    failures return -ENOMEM or other similar values.

    A simplified version of the semantic match that finds this problem is as
    follows: (http://coccinelle.lip6.fr/)

    //
    @@
    expression ret;
    expression x,e1,e2,e3;
    @@

    ret = 0
    ... when != ret = e1
    *x = \(kmalloc\|kcalloc\|kzalloc\)(...)
    ... when != ret = e2
    if (x == NULL) { ... when != ret = e3
    return ret;
    }
    //

    Signed-off-by: Julia Lawall
    Signed-off-by: Tyler Hicks

    Julia Lawall
     

11 Aug, 2010

2 commits

  • * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ecryptfs/ecryptfs-2.6:
    ecryptfs: dont call lookup_one_len to avoid NULL nameidata
    fs/ecryptfs/file.c: introduce missing free
    ecryptfs: release reference to lower mount if interpose fails
    eCryptfs: Handle ioctl calls with unlocked and compat functions
    ecryptfs: Fix warning in ecryptfs_process_response()

    Linus Torvalds
     
  • * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6: (96 commits)
    no need for list_for_each_entry_safe()/resetting with superblock list
    Fix sget() race with failing mount
    vfs: don't hold s_umount over close_bdev_exclusive() call
    sysv: do not mark superblock dirty on remount
    sysv: do not mark superblock dirty on mount
    btrfs: remove junk sb_dirt change
    BFS: clean up the superblock usage
    AFFS: wait for sb synchronization when needed
    AFFS: clean up dirty flag usage
    cifs: truncate fallout
    mbcache: fix shrinker function return value
    mbcache: Remove unused features
    add f_flags to struct statfs(64)
    pass a struct path to vfs_statfs
    update VFS documentation for method changes.
    All filesystems that need invalidate_inode_buffers() are doing that explicitly
    convert remaining ->clear_inode() to ->evict_inode()
    Make ->drop_inode() just return whether inode needs to be dropped
    fs/inode.c:clear_inode() is gone
    fs/inode.c:evict() doesn't care about delete vs. non-delete paths now
    ...

    Fix up trivial conflicts in fs/nilfs2/super.c

    Linus Torvalds
     

10 Aug, 2010

5 commits

  • We'll need the path to implement the flags field for statvfs support.
    We do have it available in all callers except:

    - ecryptfs_statfs. This one doesn't actually need vfs_statfs but just
    needs to do a caller to the lower filesystem statfs method.
    - sys_ustat. Add a non-exported statfs_by_dentry helper for it which
    doesn't won't be able to fill out the flags field later on.

    In addition rename the helpers for statfs vs fstatfs to do_*statfs instead
    of the misleading vfs prefix.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Al Viro

    Christoph Hellwig
     
  • Signed-off-by: Al Viro

    Al Viro
     
  • Make sure we check the truncate constraints early on in ->setattr by adding
    those checks to inode_change_ok. Also clean up and document inode_change_ok
    to make this obvious.

    As a fallout we don't have to call inode_newsize_ok from simple_setsize and
    simplify it down to a truncate_setsize which doesn't return an error. This
    simplifies a lot of setattr implementations and means we use truncate_setsize
    almost everywhere. Get rid of fat_setsize now that it's trivial and mark
    ext2_setsize static to make the calling convention obvious.

    Keep the inode_newsize_ok in vmtruncate for now as all callers need an
    audit for its removal anyway.

    Note: setattr code in ecryptfs doesn't call inode_change_ok at all and
    needs a deeper audit, but that is left for later.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Al Viro

    Christoph Hellwig
     
  • I have encountered the same problem that Eric Sandeen described in
    this post

    http://lkml.org/lkml/fancy/2010/4/23/467

    while experimenting with stackable filesystems.

    The reason seems to be that ecryptfs calls lookup_one_len() to get the
    lower dentry, which in turn calls the lower parent dirs d_revalidate()
    with a NULL nameidata object.
    If ecryptfs is the underlaying filesystem, the NULL pointer dereference
    occurs, since ecryptfs is not prepared to handle a NULL nameidata.

    I know that this cant happen any more, since it is no longer allowed to
    mount ecryptfs upon itself.

    But maybe this patch it useful nevertheless, since the problem would still
    apply for an underlaying filesystem that implements d_revalidate() and is
    not prepared to handle a NULL nameidata (I dont know if there actually
    is such a fs).

    With this patch (against 2.6.35-rc5) ecryptfs uses the vfs_lookup_path()
    function instead of lookup_one_len() which ensures that the nameidata
    passed to the lower filesystems d_revalidate().

    Signed-off-by: Lino Sanfilippo
    Signed-off-by: Tyler Hicks

    Lino Sanfilippo
     
  • The comments in the code indicate that file_info should be released if the
    function fails. This releasing is done at the label out_free, not out.

    The semantic match that finds this problem is as follows:
    (http://www.emn.fr/x-info/coccinelle/)

    //
    @r exists@
    local idexpression x;
    statement S;
    expression E;
    identifier f,f1,l;
    position p1,p2;
    expression *ptr != NULL;
    @@

    x@p1 = kmem_cache_zalloc(...);
    ...
    if (x == NULL) S
    }
    (
    x->f1 = E
    |
    (x->f1 == NULL || ...)
    |
    f(...,x->f1,...)
    )
    ...>
    (
    return ;
    |
    return@p2 ...;
    )

    @script:python@
    p1 << r.p1;
    p2 << r.p2;
    @@

    print "* file: %s kmem_cache_zalloc %s" % (p1[0].file,p1[0].line)
    //

    Signed-off-by: Julia Lawall
    Cc: stable@kernel.org
    Signed-off-by: Tyler Hicks

    Julia Lawall
     

09 Aug, 2010

3 commits

  • In ecryptfs_lookup_and_interpose_lower() the lower mount is not decremented
    if allocation of a dentry info struct failed. As a result the lower filesystem
    cant be unmounted any more (since it is considered busy). This patch corrects
    the reference counting.

    Signed-off-by: Lino Sanfilippo
    Cc: stable@kernel.org
    Signed-off-by: Tyler Hicks

    Lino Sanfilippo
     
  • Lower filesystems that only implemented unlocked_ioctl weren't being
    passed ioctl calls because eCryptfs only checked for
    lower_file->f_op->ioctl and returned -ENOTTY if it was NULL.

    eCryptfs shouldn't implement ioctl(), since it doesn't require the BKL.
    This patch introduces ecryptfs_unlocked_ioctl() and
    ecryptfs_compat_ioctl(), which passes the calls on to the lower file
    system.

    https://bugs.launchpad.net/ecryptfs/+bug/469664

    Reported-by: James Dupin
    Cc: stable@kernel.org
    Signed-off-by: Tyler Hicks

    Tyler Hicks
     
  • Fix warning seen with "make -j24 CONFIG_DEBUG_SECTION_MISMATCH=y V=1":

    fs/ecryptfs/messaging.c: In function 'ecryptfs_process_response':
    fs/ecryptfs/messaging.c:276: warning: 'daemon' may be used uninitialized in this function

    Signed-off-by: Prarit Bhargava
    Signed-off-by: Tyler Hicks

    Prarit Bhargava
     

04 Aug, 2010

1 commit


29 Jul, 2010

1 commit

  • The function ecryptfs_uid_hash wrongly assumes that the
    second parameter to hash_long() is the number of hash
    buckets instead of the number of hash bits.
    This patch fixes that and renames the variable
    ecryptfs_hash_buckets to ecryptfs_hash_bits to make it
    clearer.

    Fixes: CVE-2010-2492

    Signed-off-by: Andre Osterhues
    Signed-off-by: Tyler Hicks
    Signed-off-by: Linus Torvalds

    Andre Osterhues
     

17 Jun, 2010

2 commits


28 May, 2010

2 commits


22 May, 2010

7 commits


22 Apr, 2010

1 commit


20 Apr, 2010

6 commits

  • * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ecryptfs/ecryptfs-2.6:
    eCryptfs: Turn lower lookup error messages into debug messages
    eCryptfs: Copy lower directory inode times and size on link
    ecryptfs: fix use with tmpfs by removing d_drop from ecryptfs_destroy_inode
    ecryptfs: fix error code for missing xattrs in lower fs
    eCryptfs: Decrypt symlink target for stat size
    eCryptfs: Strip metadata in xattr flag in encrypted view
    eCryptfs: Clear buffer before reading in metadata xattr
    eCryptfs: Rename ecryptfs_crypt_stat.num_header_bytes_at_front
    eCryptfs: Fix metadata in xattr feature regression

    Linus Torvalds
     
  • Vaugue warnings about ENAMETOOLONG errors when looking up an encrypted
    file name have caused many users to become concerned about their data.
    Since this is a rather harmless condition, I'm moving this warning to
    only be printed when the ecryptfs_verbosity module param is 1.

    Signed-off-by: Tyler Hicks

    Tyler Hicks
     
  • The timestamps and size of a lower inode involved in a link() call was
    being copied to the upper parent inode. Instead, we should be
    copying lower parent inode's timestamps and size to the upper parent
    inode. I discovered this bug using the POSIX test suite at Tuxera.

    Signed-off-by: Tyler Hicks

    Tyler Hicks
     
  • Since tmpfs has no persistent storage, it pins all its dentries in memory
    so they have d_count=1 when other file systems would have d_count=0.
    ->lookup is only used to create new dentries. If the caller doesn't
    instantiate it, it's freed immediately at dput(). ->readdir reads
    directly from the dcache and depends on the dentries being hashed.

    When an ecryptfs mount is mounted, it associates the lower file and dentry
    with the ecryptfs files as they're accessed. When it's umounted and
    destroys all the in-memory ecryptfs inodes, it fput's the lower_files and
    d_drop's the lower_dentries. Commit 4981e081 added this and a d_delete in
    2008 and several months later commit caeeeecf removed the d_delete. I
    believe the d_drop() needs to be removed as well.

    The d_drop effectively hides any file that has been accessed via ecryptfs
    from the underlying tmpfs since it depends on it being hashed for it to
    be accessible. I've removed the d_drop on my development node and see no
    ill effects with basic testing on both tmpfs and persistent storage.

    As a side effect, after ecryptfs d_drops the dentries on tmpfs, tmpfs
    BUGs on umount. This is due to the dentries being unhashed.
    tmpfs->kill_sb is kill_litter_super which calls d_genocide to drop
    the reference pinning the dentry. It skips unhashed and negative dentries,
    but shrink_dcache_for_umount_subtree doesn't. Since those dentries
    still have an elevated d_count, we get a BUG().

    This patch removes the d_drop call and fixes both issues.

    This issue was reported at:
    https://bugzilla.novell.com/show_bug.cgi?id=567887

    Reported-by: Árpád Bíró
    Signed-off-by: Jeff Mahoney
    Cc: Dustin Kirkland
    Cc: stable@kernel.org
    Signed-off-by: Tyler Hicks

    Jeff Mahoney
     
  • If the lower file system driver has extended attributes disabled,
    ecryptfs' own access functions return -ENOSYS instead of -EOPNOTSUPP.
    This breaks execution of programs in the ecryptfs mount, since the
    kernel expects the latter error when checking for security
    capabilities in xattrs.

    Signed-off-by: Christian Pulvermacher
    Cc: stable@kernel.org
    Signed-off-by: Tyler Hicks

    Christian Pulvermacher
     
  • Create a getattr handler for eCryptfs symlinks that is capable of
    reading the lower target and decrypting its path. Prior to this patch,
    a stat's st_size field would represent the strlen of the encrypted path,
    while readlink() would return the strlen of the decrypted path. This
    could lead to confusion in some userspace applications, since the two
    values should be equal.

    https://bugs.launchpad.net/bugs/524919

    Reported-by: Loïc Minier
    Cc: stable@kernel.org
    Signed-off-by: Tyler Hicks

    Tyler Hicks
     

30 Mar, 2010

1 commit

  • …it slab.h inclusion from percpu.h

    percpu.h is included by sched.h and module.h and thus ends up being
    included when building most .c files. percpu.h includes slab.h which
    in turn includes gfp.h making everything defined by the two files
    universally available and complicating inclusion dependencies.

    percpu.h -> slab.h dependency is about to be removed. Prepare for
    this change by updating users of gfp and slab facilities include those
    headers directly instead of assuming availability. As this conversion
    needs to touch large number of source files, the following script is
    used as the basis of conversion.

    http://userweb.kernel.org/~tj/misc/slabh-sweep.py

    The script does the followings.

    * Scan files for gfp and slab usages and update includes such that
    only the necessary includes are there. ie. if only gfp is used,
    gfp.h, if slab is used, slab.h.

    * When the script inserts a new include, it looks at the include
    blocks and try to put the new include such that its order conforms
    to its surrounding. It's put in the include block which contains
    core kernel includes, in the same order that the rest are ordered -
    alphabetical, Christmas tree, rev-Xmas-tree or at the end if there
    doesn't seem to be any matching order.

    * If the script can't find a place to put a new include (mostly
    because the file doesn't have fitting include block), it prints out
    an error message indicating which .h file needs to be added to the
    file.

    The conversion was done in the following steps.

    1. The initial automatic conversion of all .c files updated slightly
    over 4000 files, deleting around 700 includes and adding ~480 gfp.h
    and ~3000 slab.h inclusions. The script emitted errors for ~400
    files.

    2. Each error was manually checked. Some didn't need the inclusion,
    some needed manual addition while adding it to implementation .h or
    embedding .c file was more appropriate for others. This step added
    inclusions to around 150 files.

    3. The script was run again and the output was compared to the edits
    from #2 to make sure no file was left behind.

    4. Several build tests were done and a couple of problems were fixed.
    e.g. lib/decompress_*.c used malloc/free() wrappers around slab
    APIs requiring slab.h to be added manually.

    5. The script was run on all .h files but without automatically
    editing them as sprinkling gfp.h and slab.h inclusions around .h
    files could easily lead to inclusion dependency hell. Most gfp.h
    inclusion directives were ignored as stuff from gfp.h was usually
    wildly available and often used in preprocessor macros. Each
    slab.h inclusion directive was examined and added manually as
    necessary.

    6. percpu.h was updated not to include slab.h.

    7. Build test were done on the following configurations and failures
    were fixed. CONFIG_GCOV_KERNEL was turned off for all tests (as my
    distributed build env didn't work with gcov compiles) and a few
    more options had to be turned off depending on archs to make things
    build (like ipr on powerpc/64 which failed due to missing writeq).

    * x86 and x86_64 UP and SMP allmodconfig and a custom test config.
    * powerpc and powerpc64 SMP allmodconfig
    * sparc and sparc64 SMP allmodconfig
    * ia64 SMP allmodconfig
    * s390 SMP allmodconfig
    * alpha SMP allmodconfig
    * um on x86_64 SMP allmodconfig

    8. percpu.h modifications were reverted so that it could be applied as
    a separate patch and serve as bisection point.

    Given the fact that I had only a couple of failures from tests on step
    6, I'm fairly confident about the coverage of this conversion patch.
    If there is a breakage, it's likely to be something in one of the arch
    headers which should be easily discoverable easily on most builds of
    the specific arch.

    Signed-off-by: Tejun Heo <tj@kernel.org>
    Guess-its-ok-by: Christoph Lameter <cl@linux-foundation.org>
    Cc: Ingo Molnar <mingo@redhat.com>
    Cc: Lee Schermerhorn <Lee.Schermerhorn@hp.com>

    Tejun Heo
     

24 Mar, 2010

4 commits

  • The ecryptfs_encrypted_view mount option provides a unified way of
    viewing encrypted eCryptfs files. If the metadata is stored in a xattr,
    the metadata is moved to the file header when the file is read inside
    the eCryptfs mount. Because of this, we should strip the
    ECRYPTFS_METADATA_IN_XATTR flag from the header's flag section. This
    allows eCryptfs to treat the file as an eCryptfs file with a header
    at the front.

    Reviewed-by: Eric Sandeen
    Signed-off-by: Tyler Hicks

    Tyler Hicks
     
  • We initially read in the first PAGE_CACHE_SIZE of a file to if the
    eCryptfs header marker can be found. If it isn't found and
    ecryptfs_xattr_metadata was given as a mount option, then the
    user.ecryptfs xattr is read into the same buffer. Since the data from
    the first page of the file wasn't cleared, it is possible that we think
    we've found a second tag 3 or tag 1 packet and then error out after the
    packet contents aren't as expected. This patch clears the buffer before
    filling it with metadata from the user.ecryptfs xattr.

    Reviewed-by: Eric Sandeen
    Signed-off-by: Tyler Hicks

    Tyler Hicks
     
  • This patch renames the num_header_bytes_at_front variable to
    metadata_size since it now contains the max size of the metadata.

    Reviewed-by: Eric Sandeen
    Signed-off-by: Tyler Hicks

    Tyler Hicks
     
  • Fixes regression in 8faece5f906725c10e7a1f6caf84452abadbdc7b

    When using the ecryptfs_xattr_metadata mount option, eCryptfs stores the
    metadata (normally stored at the front of the file) in the user.ecryptfs
    xattr. This causes ecryptfs_crypt_stat.num_header_bytes_at_front to be
    0, since there is no header data at the front of the file. This results
    in too much memory being requested and ENOMEM being returned from
    ecryptfs_write_metadata().

    This patch fixes the problem by using the num_header_bytes_at_front
    variable for specifying the max size of the metadata, despite whether it
    is stored in the header or xattr.

    Reviewed-by: Eric Sandeen
    Signed-off-by: Tyler Hicks

    Tyler Hicks
     

20 Jan, 2010

2 commits

  • The "full_alg_name" variable is used on a couple error paths, so we
    shouldn't free it until the end.

    Signed-off-by: Dan Carpenter
    Cc: stable@kernel.org
    Signed-off-by: Tyler Hicks

    Dan Carpenter
     
  • The variable lower_dentry is initialized twice to the same (side effect-free)
    expression. Drop one initialization.

    A simplified version of the semantic match that finds this problem is:
    (http://coccinelle.lip6.fr/)

    //
    @forall@
    idexpression *x;
    identifier f!=ERR_PTR;
    @@

    x = f(...)
    ... when != x
    (
    x = f(...,,...)
    |
    * x = f(...)
    )
    //

    Signed-off-by: Julia Lawall
    Signed-off-by: Tyler Hicks

    Julia Lawall