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

3 commits


14 Jul, 2012

7 commits

  • Pass mount flags to sget() so that it can use them in initialising a new
    superblock before the set function is called. They could also be passed to the
    compare function.

    Signed-off-by: David Howells
    Signed-off-by: Al Viro

    David Howells
     
  • 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
     
  • Just the lookup flags. Die, bastard, die...

    Signed-off-by: Al Viro

    Al Viro
     
  • The issue occurs when eCryptfs is mounted with a cipher supported by
    the crypto subsystem but not by eCryptfs. The mount succeeds and an
    error does not occur until a write. This change checks for eCryptfs
    cipher support at mount time.

    Resolves Launchpad issue #338914, reported by Tyler Hicks in 03/2009.
    https://bugs.launchpad.net/ecryptfs/+bug/338914

    Signed-off-by: Tim Sally
    Signed-off-by: Tyler Hicks

    Tim Sally
     
  • 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

5 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
     
  • Now that a pointer to a valid struct ecryptfs_daemon is stored in the
    private_data of an opened /dev/ecryptfs file, the remaining miscdev
    functions can utilize the pointer rather than looking up the
    ecryptfs_daemon at the beginning of each operation.

    The security model of /dev/ecryptfs is simplified a little bit with this
    patch. Upon opening /dev/ecryptfs, a per-user ecryptfs_daemon is
    registered. Another daemon cannot be registered for that user until the
    last file reference is released. During the lifetime of the
    ecryptfs_daemon, access checks are not performed on the /dev/ecryptfs
    operations because it is assumed that the application securely handles
    the opened file descriptor and does not unintentionally leak it to
    processes that are not trusted.

    Signed-off-by: Tyler Hicks
    Cc: Sasha Levin

    Tyler Hicks
     
  • These are no longer needed.

    Signed-off-by: Tyler Hicks
    Cc: Sasha Levin

    Tyler Hicks
     
  • When the eCryptfs mount options do not include '-o acl', but the lower
    filesystem's mount options do include 'acl', the MS_POSIXACL flag is not
    flipped on in the eCryptfs super block flags. This flag is what the VFS
    checks in do_last() when deciding if the current umask should be applied
    to a newly created inode's mode or not. When a default POSIX ACL mask is
    set on a directory, the current umask is incorrectly applied to new
    inodes created in the directory. This patch ignores the MS_POSIXACL flag
    passed into ecryptfs_mount() and sets the flag on the eCryptfs super
    block depending on the flag's presence on the lower super block.

    Additionally, it is incorrect to allow a writeable eCryptfs mount on top
    of a read-only lower mount. This missing check did not allow writes to
    the read-only lower mount because permissions checks are still performed
    on the lower filesystem's objects but it is best to simply not allow a
    rw mount on top of ro mount. However, a ro eCryptfs mount on top of a rw
    mount is valid and still allowed.

    https://launchpad.net/bugs/1009207

    Signed-off-by: Tyler Hicks
    Reported-by: Stefan Beller
    Cc: John Johansen

    Tyler Hicks
     

07 Jul, 2012

1 commit

  • File operations on /dev/ecryptfs would BUG() when the operations were
    performed by processes other than the process that originally opened the
    file. This could happen with open files inherited after fork() or file
    descriptors passed through IPC mechanisms. Rather than calling BUG(), an
    error code can be safely returned in most situations.

    In ecryptfs_miscdev_release(), eCryptfs still needs to handle the
    release even if the last file reference is being held by a process that
    didn't originally open the file. ecryptfs_find_daemon_by_euid() will not
    be successful, so a pointer to the daemon is stored in the file's
    private_data. The private_data pointer is initialized when the miscdev
    file is opened and only used when the file is released.

    https://launchpad.net/bugs/994247

    Signed-off-by: Tyler Hicks
    Reported-by: Sasha Levin
    Tested-by: Sasha Levin

    Tyler Hicks
     

04 Jul, 2012

2 commits

  • Don't grab the daemon mutex while holding the message context mutex.
    Addresses this lockdep warning:

    ecryptfsd/2141 is trying to acquire lock:
    (&ecryptfs_msg_ctx_arr[i].mux){+.+.+.}, at: [] ecryptfs_miscdev_read+0x143/0x470 [ecryptfs]

    but task is already holding lock:
    (&(*daemon)->mux){+.+...}, at: [] ecryptfs_miscdev_read+0x21c/0x470 [ecryptfs]

    which lock already depends on the new lock.

    the existing dependency chain (in reverse order) is:

    -> #1 (&(*daemon)->mux){+.+...}:
    [] lock_acquire+0x9d/0x220
    [] __mutex_lock_common+0x5a/0x4b0
    [] mutex_lock_nested+0x44/0x50
    [] ecryptfs_send_miscdev+0x97/0x120 [ecryptfs]
    [] ecryptfs_send_message+0x134/0x1e0 [ecryptfs]
    [] ecryptfs_generate_key_packet_set+0x2fe/0xa80 [ecryptfs]
    [] ecryptfs_write_metadata+0x108/0x250 [ecryptfs]
    [] ecryptfs_create+0x130/0x250 [ecryptfs]
    [] vfs_create+0xb4/0x120
    [] do_last+0x8c5/0xa10
    [] path_openat+0xd9/0x460
    [] do_filp_open+0x42/0xa0
    [] do_sys_open+0xf8/0x1d0
    [] sys_open+0x21/0x30
    [] system_call_fastpath+0x16/0x1b

    -> #0 (&ecryptfs_msg_ctx_arr[i].mux){+.+.+.}:
    [] __lock_acquire+0x1bf8/0x1c50
    [] lock_acquire+0x9d/0x220
    [] __mutex_lock_common+0x5a/0x4b0
    [] mutex_lock_nested+0x44/0x50
    [] ecryptfs_miscdev_read+0x143/0x470 [ecryptfs]
    [] vfs_read+0xb3/0x180
    [] sys_read+0x4d/0x90
    [] system_call_fastpath+0x16/0x1b

    Signed-off-by: Tyler Hicks

    Tyler Hicks
     
  • If the first attempt at opening the lower file read/write fails,
    eCryptfs will retry using a privileged kthread. However, the privileged
    retry should not happen if the lower file's inode is read-only because a
    read/write open will still be unsuccessful.

    The check for determining if the open should be retried was intended to
    be based on the access mode of the lower file's open flags being
    O_RDONLY, but the check was incorrectly performed. This would cause the
    open to be retried by the privileged kthread, resulting in a second
    failed open of the lower file. This patch corrects the check to
    determine if the open request should be handled by the privileged
    kthread.

    Signed-off-by: Tyler Hicks
    Reported-by: Dan Carpenter
    Acked-by: Dan Carpenter

    Tyler Hicks
     

30 May, 2012

1 commit


29 May, 2012

1 commit

  • Pull writeback tree from Wu Fengguang:
    "Mainly from Jan Kara to avoid iput() in the flusher threads."

    * tag 'writeback' of git://git.kernel.org/pub/scm/linux/kernel/git/wfg/linux:
    writeback: Avoid iput() from flusher thread
    vfs: Rename end_writeback() to clear_inode()
    vfs: Move waiting for inode writeback from end_writeback() to evict_inode()
    writeback: Refactor writeback_single_inode()
    writeback: Remove wb->list_lock from writeback_single_inode()
    writeback: Separate inode requeueing after writeback
    writeback: Move I_DIRTY_PAGES handling
    writeback: Move requeueing when I_SYNC set to writeback_sb_inodes()
    writeback: Move clearing of I_SYNC into inode_sync_complete()
    writeback: initialize global_dirty_limit
    fs: remove 8 bytes of padding from struct writeback_control on 64 bit builds
    mm: page-writeback.c: local functions should not be exposed globally

    Linus Torvalds
     

06 May, 2012

1 commit

  • After we moved inode_sync_wait() from end_writeback() it doesn't make sense
    to call the function end_writeback() anymore. Rename it to clear_inode()
    which well says what the function really does - set I_CLEAR flag.

    Signed-off-by: Jan Kara
    Signed-off-by: Fengguang Wu

    Jan Kara
     

08 Apr, 2012

1 commit


21 Mar, 2012

4 commits


29 Feb, 2012

1 commit

  • Fix printk format warning (from Linus's suggestion):

    on i386:
    fs/ecryptfs/miscdev.c:433:38: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'unsigned int'

    and on x86_64:
    fs/ecryptfs/miscdev.c:433:38: warning: format '%u' expects type 'unsigned int', but argument 4 has type 'long unsigned int'

    Signed-off-by: Randy Dunlap
    Cc: Geert Uytterhoeven
    Cc: Tyler Hicks
    Cc: Dustin Kirkland
    Cc: ecryptfs@vger.kernel.org
    Signed-off-by: Linus Torvalds

    Randy Dunlap
     

17 Feb, 2012

3 commits

  • Signed-off-by: Cong Wang
    Signed-off-by: Tyler Hicks

    Cong Wang
     
  • 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
     
  • statfs() calls on eCryptfs files returned the wrong filesystem type and,
    when using filename encryption, the wrong maximum filename length.

    If mount-wide filename encryption is enabled, the cipher block size and
    the lower filesystem's max filename length will determine the max
    eCryptfs filename length. Pre-tested, known good lengths are used when
    the lower filesystem's namelen is 255 and a cipher with 8 or 16 byte
    block sizes is used. In other, less common cases, we fall back to a safe
    rounded-down estimate when determining the eCryptfs namelen.

    https://launchpad.net/bugs/885744

    Signed-off-by: Tyler Hicks
    Reported-by: Kees Cook
    Reviewed-by: Kees Cook
    Reviewed-by: John Johansen

    Tyler Hicks
     

26 Jan, 2012

7 commits

  • The data encryption was moved from ecryptfs_write_end into
    ecryptfs_writepage, this patch moves the corresponding function
    comments to be consistent with the modification.

    Signed-off-by: Li Wang
    Signed-off-by: Linus Torvalds

    Li Wang
     
  • If pages passed to the eCryptfs extent-based crypto functions are not
    mapped and the module parameter ecryptfs_verbosity=1 was specified at
    loading time, a NULL pointer dereference will occur.

    Note that this wouldn't happen on a production system, as you wouldn't
    pass ecryptfs_verbosity=1 on a production system. It leaks private
    information to the system logs and is for debugging only.

    The debugging info printed in these messages is no longer very useful
    and rather than doing a kmap() in these debugging paths, it will be
    better to simply remove the debugging paths completely.

    https://launchpad.net/bugs/913651

    Signed-off-by: Tyler Hicks
    Reported-by: Daniel DeFreez
    Cc:

    Tyler Hicks
     
  • ecryptfs_read() has been ifdef'ed out for years now and it was
    apparently unused before then. It is time to get rid of it for good.

    Signed-off-by: Tyler Hicks

    Tyler Hicks
     
  • 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
     
  • ecryptfs_write() handles the truncation of eCryptfs inodes. It grabs a
    page, zeroes out the appropriate portions, and then encrypts the page
    before writing it to the lower filesystem. It was unkillable and due to
    the lack of sparse file support could result in tying up a large portion
    of system resources, while encrypting pages of zeros, with no way for
    the truncate operation to be stopped from userspace.

    This patch adds the ability for ecryptfs_write() to detect a pending
    fatal signal and return as gracefully as possible. The intent is to
    leave the lower file in a useable state, while still allowing a user to
    break out of the encryption loop. If a pending fatal signal is detected,
    the eCryptfs inode size is updated to reflect the modified inode size
    and then -EINTR is returned.

    Signed-off-by: Tyler Hicks
    Cc:

    Tyler Hicks
     
  • ecryptfs_write() can enter an infinite loop when truncating a file to a
    size larger than 4G. This only happens on architectures where size_t is
    represented by 32 bits.

    This was caused by a size_t overflow due to it incorrectly being used to
    store the result of a calculation which uses potentially large values of
    type loff_t.

    [tyhicks@canonical.com: rewrite subject and commit message]
    Signed-off-by: Li Wang
    Signed-off-by: Yunchuan Wen
    Reviewed-by: Cong Wang
    Cc:
    Signed-off-by: Tyler Hicks

    Li Wang
     
  • ecryptfs_miscdev_read() and ecryptfs_miscdev_write() contained many
    magic numbers for specifying packet header field sizes and offsets. This
    patch defines those values and replaces the magic values.

    Signed-off-by: Tyler Hicks

    Tyler Hicks