08 Mar, 2013

1 commit

  • Pull ecryptfs fixes from Tyler Hicks:
    "Minor code cleanups and new Kconfig option to disable /dev/ecryptfs

    The code cleanups fix up W=1 compiler warnings and some unnecessary
    checks. The new Kconfig option, defaulting to N, allows the rarely
    used eCryptfs kernel to userspace communication channel to be compiled
    out. This may be the first step in it being eventually removed."

    Hmm. I'm not sure whether these should be called "fixes", and it
    probably should have gone in the merge window. But I'll let it slide.

    * tag 'ecryptfs-3.9-rc2-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/tyhicks/ecryptfs:
    eCryptfs: allow userspace messaging to be disabled
    eCryptfs: Fix redundant error check on ecryptfs_find_daemon_by_euid()
    ecryptfs: ecryptfs_msg_ctx_alloc_to_free(): remove kfree() redundant null check
    eCryptfs: decrypt_pki_encrypted_session_key(): remove kfree() redundant null check
    eCryptfs: remove unneeded checks in virt_to_scatterlist()
    eCryptfs: Fix -Wmissing-prototypes warnings
    eCryptfs: Fix -Wunused-but-set-variable warnings
    eCryptfs: initialize payload_len in keystore.c

    Linus Torvalds
     

26 Feb, 2013

1 commit


18 Jan, 2013

1 commit


15 Sep, 2012

1 commit

  • After calling into the lower filesystem to do a rename, the lower target
    inode's attributes were not copied up to the eCryptfs target inode. This
    resulted in the eCryptfs target inode staying around, rather than being
    evicted, because i_nlink was not updated for the eCryptfs inode. This
    also meant that eCryptfs didn't do the final iput() on the lower target
    inode so it stayed around, as well. This would result in a failure to
    free up space occupied by the target file in the rename() operation.
    Both target inodes would eventually be evicted when the eCryptfs
    filesystem was unmounted.

    This patch calls fsstack_copy_attr_all() after the lower filesystem
    does its ->rename() so that important inode attributes, such as i_nlink,
    are updated at the eCryptfs layer. ecryptfs_evict_inode() is now called
    and eCryptfs can drop its final reference on the lower inode.

    http://launchpad.net/bugs/561129

    Signed-off-by: Tyler Hicks
    Tested-by: Colin Ian King
    Cc: [2.6.39+]

    Tyler Hicks
     

03 Aug, 2012

1 commit

  • Pull ecryptfs fixes from Tyler Hicks:
    - Fixes a bug when the lower filesystem mount options include 'acl',
    but the eCryptfs mount options do not
    - Cleanups in the messaging code
    - Better handling of empty files in the lower filesystem to improve
    usability. Failed file creations are now cleaned up and empty lower
    files are converted into eCryptfs during open().
    - The write-through cache changes are being reverted due to bugs that
    are not easy to fix. Stability outweighs the performance
    enhancements here.
    - Improvement to the mount code to catch unsupported ciphers specified
    in the mount options

    * tag 'ecryptfs-3.6-rc1-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/tyhicks/ecryptfs:
    eCryptfs: check for eCryptfs cipher support at mount
    eCryptfs: Revert to a writethrough cache model
    eCryptfs: Initialize empty lower files when opening them
    eCryptfs: Unlink lower inode when ecryptfs_create() fails
    eCryptfs: Make all miscdev functions use daemon ptr in file private_data
    eCryptfs: Remove unused messaging declarations and function
    eCryptfs: Copy up POSIX ACL and read-only flags from lower mount

    Linus Torvalds
     

30 Jul, 2012

2 commits


23 Jul, 2012

1 commit


14 Jul, 2012

4 commits

  • all we want is a boolean flag, same as the method gets now

    Signed-off-by: Al Viro

    Al Viro
     
  • boolean "does it have to be exclusive?" flag is passed instead;
    Local filesystem should just ignore it - the object is guaranteed
    not to be there yet.

    Signed-off-by: Al Viro

    Al Viro
     
  • Just the flags; only NFS cares even about that, but there are
    legitimate uses for such argument. And getting rid of that
    completely would require splitting ->lookup() into a couple
    of methods (at least), so let's leave that alone for now...

    Signed-off-by: Al Viro

    Al Viro
     
  • A change was made about a year ago to get eCryptfs to better utilize its
    page cache during writes. The idea was to do the page encryption
    operations during page writeback, rather than doing them when initially
    writing into the page cache, to reduce the number of page encryption
    operations during sequential writes. This meant that the encrypted page
    would only be written to the lower filesystem during page writeback,
    which was a change from how eCryptfs had previously wrote to the lower
    filesystem in ecryptfs_write_end().

    The change caused a few eCryptfs-internal bugs that were shook out.
    Unfortunately, more grave side effects have been identified that will
    force changes outside of eCryptfs. Because the lower filesystem isn't
    consulted until page writeback, eCryptfs has no way to pass lower write
    errors (ENOSPC, mainly) back to userspace. Additionaly, it was reported
    that quotas could be bypassed because of the way eCryptfs may sometimes
    open the lower filesystem using a privileged kthread.

    It would be nice to resolve the latest issues, but it is best if the
    eCryptfs commits be reverted to the old behavior in the meantime.

    This reverts:
    32001d6f "eCryptfs: Flush file in vma close"
    5be79de2 "eCryptfs: Flush dirty pages in setattr"
    57db4e8d "ecryptfs: modify write path to encrypt page in writepage"

    Signed-off-by: Tyler Hicks
    Tested-by: Colin King
    Cc: Colin King
    Cc: Thieu Le

    Tyler Hicks
     

09 Jul, 2012

2 commits

  • Historically, eCryptfs has only initialized lower files in the
    ecryptfs_create() path. Lower file initialization is the act of writing
    the cryptographic metadata from the inode's crypt_stat to the header of
    the file. The ecryptfs_open() path already expects that metadata to be
    in the header of the file.

    A number of users have reported empty lower files in beneath their
    eCryptfs mounts. Most of the causes for those empty files being left
    around have been addressed, but the presence of empty files causes
    problems due to the lack of proper cryptographic metadata.

    To transparently solve this problem, this patch initializes empty lower
    files in the ecryptfs_open() error path. If the metadata is unreadable
    due to the lower inode size being 0, plaintext passthrough support is
    not in use, and the metadata is stored in the header of the file (as
    opposed to the user.ecryptfs extended attribute), the lower file will be
    initialized.

    The number of nested conditionals in ecryptfs_open() was getting out of
    hand, so a helper function was created. To avoid the same nested
    conditional problem, the conditional logic was reversed inside of the
    helper function.

    https://launchpad.net/bugs/911507

    Signed-off-by: Tyler Hicks
    Cc: John Johansen
    Cc: Colin Ian King

    Tyler Hicks
     
  • ecryptfs_create() creates a lower inode, allocates an eCryptfs inode,
    initializes the eCryptfs inode and cryptographic metadata attached to
    the inode, and then writes the metadata to the header of the file.

    If an error was to occur after the lower inode was created, an empty
    lower file would be left in the lower filesystem. This is a problem
    because ecryptfs_open() refuses to open any lower files which do not
    have the appropriate metadata in the file header.

    This patch properly unlinks the lower inode when an error occurs in the
    later stages of ecryptfs_create(), reducing the chance that an empty
    lower file will be left in the lower filesystem.

    https://launchpad.net/bugs/872905

    Signed-off-by: Tyler Hicks
    Cc: John Johansen
    Cc: Colin Ian King

    Tyler Hicks
     

30 May, 2012

1 commit


17 Feb, 2012

1 commit

  • After passing through a ->setxattr() call, eCryptfs needs to copy the
    inode attributes from the lower inode to the eCryptfs inode, as they
    may have changed in the lower filesystem's ->setxattr() path.

    One example is if an extended attribute containing a POSIX Access
    Control List is being set. The new ACL may cause the lower filesystem to
    modify the mode of the lower inode and the eCryptfs inode would need to
    be updated to reflect the new mode.

    https://launchpad.net/bugs/926292

    Signed-off-by: Tyler Hicks
    Reported-by: Sebastien Bacher
    Cc: John Johansen
    Cc:

    Tyler Hicks
     

26 Jan, 2012

1 commit

  • Most filesystems call inode_change_ok() very early in ->setattr(), but
    eCryptfs didn't call it at all. It allowed the lower filesystem to make
    the call in its ->setattr() function. Then, eCryptfs would copy the
    appropriate inode attributes from the lower inode to the eCryptfs inode.

    This patch changes that and actually calls inode_change_ok() on the
    eCryptfs inode, fairly early in ecryptfs_setattr(). Ideally, the call
    would happen earlier in ecryptfs_setattr(), but there are some possible
    inode initialization steps that must happen first.

    Since the call was already being made on the lower inode, the change in
    functionality should be minimal, except for the case of a file extending
    truncate call. In that case, inode_newsize_ok() was never being
    called on the eCryptfs inode. Rather than inode_newsize_ok() catching
    maximum file size errors early on, eCryptfs would encrypt zeroed pages
    and write them to the lower filesystem until the lower filesystem's
    write path caught the error in generic_write_checks(). This patch
    introduces a new function, called ecryptfs_inode_newsize_ok(), which
    checks if the new lower file size is within the appropriate limits when
    the truncate operation will be growing the lower file.

    In summary this change prevents eCryptfs truncate operations (and the
    resulting page encryptions), which would exceed the lower filesystem
    limits or FSIZE rlimits, from ever starting.

    Signed-off-by: Tyler Hicks
    Reviewed-by: Li Wang
    Cc:

    Tyler Hicks
     

04 Jan, 2012

5 commits


24 Nov, 2011

1 commit

  • The file creation path prematurely called d_instantiate() and
    unlock_new_inode() before the eCryptfs inode info was fully
    allocated and initialized and before the eCryptfs metadata was written
    to the lower file.

    This could result in race conditions in subsequent file and inode
    operations leading to unexpected error conditions or a null pointer
    dereference while attempting to use the unallocated memory.

    https://launchpad.net/bugs/813146

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

    Tyler Hicks
     

02 Nov, 2011

1 commit


29 Jul, 2011

1 commit


20 Jul, 2011

3 commits


30 May, 2011

4 commits

  • ecryptfs_lookup_interpose() has turned into spaghetti code over the
    years. This is an effort to clean it up.

    - Shorten overly descriptive variable names such as ecryptfs_dentry
    - Simplify gotos and error paths
    - Create helper function for reading plaintext i_size from metadata

    It also includes an optimization when reading i_size from the metadata.
    A complete page-sized kmem_cache_alloc() was being done to read in 16
    bytes of metadata. The buffer for that is now statically declared.

    Signed-off-by: Tyler Hicks

    Tyler Hicks
     
  • Only unlock and d_add() new inodes after the plaintext inode size has
    been read from the lower filesystem. This fixes a race condition that
    was sometimes seen during a multi-job kernel build in an eCryptfs mount.

    https://bugzilla.kernel.org/show_bug.cgi?id=36002

    Signed-off-by: Tyler Hicks
    Reported-by: David
    Tested-by: David

    Tyler Hicks
     
  • The eCryptfs inode get, initialization, and dentry interposition code
    has two separate paths. One is for when dentry interposition is needed
    after doing things like a mkdir in the lower filesystem and the other
    is needed after a lookup. Unlocking new inodes and doing a d_add() needs
    to happen at different times, depending on which type of dentry
    interposing is being done.

    This patch cleans up the inode get and initialization code paths and
    splits them up so that the locking and d_add() differences mentioned
    above can be handled appropriately in a later patch.

    Signed-off-by: Tyler Hicks
    Tested-by: David

    Tyler Hicks
     
  • These functions should live in inode.c since their focus is on inodes
    and they're primarily used by functions in inode.c.

    Also does a simple cleanup of ecryptfs_inode_test() and rolls
    ecryptfs_init_inode() into ecryptfs_inode_set().

    Signed-off-by: Tyler Hicks
    Tested-by: David

    Tyler Hicks
     

29 May, 2011

1 commit

  • * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6: (36 commits)
    Cache xattr security drop check for write v2
    fs: block_page_mkwrite should wait for writeback to finish
    mm: Wait for writeback when grabbing pages to begin a write
    configfs: remove unnecessary dentry_unhash on rmdir, dir rename
    fat: remove unnecessary dentry_unhash on rmdir, dir rename
    hpfs: remove unnecessary dentry_unhash on rmdir, dir rename
    minix: remove unnecessary dentry_unhash on rmdir, dir rename
    fuse: remove unnecessary dentry_unhash on rmdir, dir rename
    coda: remove unnecessary dentry_unhash on rmdir, dir rename
    afs: remove unnecessary dentry_unhash on rmdir, dir rename
    affs: remove unnecessary dentry_unhash on rmdir, dir rename
    9p: remove unnecessary dentry_unhash on rmdir, dir rename
    ncpfs: fix rename over directory with dangling references
    ncpfs: document dentry_unhash usage
    ecryptfs: remove unnecessary dentry_unhash on rmdir, dir rename
    hostfs: remove unnecessary dentry_unhash on rmdir, dir rename
    hfsplus: remove unnecessary dentry_unhash on rmdir, dir rename
    hfs: remove unnecessary dentry_unhash on rmdir, dir rename
    omfs: remove unnecessary dentry_unhash on rmdir, dir rneame
    udf: remove unnecessary dentry_unhash from rmdir, dir rename
    ...

    Linus Torvalds
     

28 May, 2011

3 commits


26 May, 2011

2 commits


26 Apr, 2011

2 commits

  • After 57db4e8d73ef2b5e94a3f412108dff2576670a8a changed eCryptfs to
    write-back caching, eCryptfs page writeback updates the lower inode
    times due to the use of vfs_write() on the lower file.

    To preserve inode metadata changes, such as 'cp -p' does with
    utimensat(), we need to flush all dirty pages early in
    ecryptfs_setattr() so that the user-updated lower inode metadata isn't
    clobbered later in writeback.

    https://bugzilla.kernel.org/show_bug.cgi?id=33372

    Reported-by: Rocko
    Signed-off-by: Tyler Hicks

    Tyler Hicks
     
  • When failing to read the lower file's crypto metadata during a lookup,
    eCryptfs must continue on without throwing an error. For example, there
    may be a plaintext file in the lower mount point that the user wants to
    delete through the eCryptfs mount.

    If an error is encountered while reading the metadata in lookup(), the
    eCryptfs inode's size could be incorrect. We must be sure to reread the
    plaintext inode size from the metadata when performing an open() or
    setattr(). The metadata is already being read in those paths, so this
    adds minimal performance overhead.

    This patch introduces a flag which will track whether or not the
    plaintext inode size has been read so that an incorrect i_size can be
    fixed in the open() or setattr() paths.

    https://bugs.launchpad.net/bugs/509180

    Cc:
    Signed-off-by: Tyler Hicks

    Tyler Hicks