24 Nov, 2011

2 commits

  • From mhalcrow's original commit message:

    Characters with ASCII values greater than the size of
    filename_rev_map[] are valid filename characters.
    ecryptfs_decode_from_filename() will access kernel memory beyond
    that array, and ecryptfs_parse_tag_70_packet() will then decrypt
    those characters. The attacker, using the FNEK of the crafted file,
    can then re-encrypt the characters to reveal the kernel memory past
    the end of the filename_rev_map[] array. I expect low security
    impact since this array is statically allocated in the text area,
    and the amount of memory past the array that is accessible is
    limited by the largest possible ASCII filename character.

    This patch solves the issue reported by mhalcrow but with an
    implementation suggested by Linus to simply extend the length of
    filename_rev_map[] to 256. Characters greater than 0x7A are mapped to
    0x00, which is how invalid characters less than 0x7A were previously
    being handled.

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

    Tyler Hicks
     
  • 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
     

30 May, 2011

4 commits

  • Now that ecryptfs_lookup_interpose() is no longer using
    ecryptfs_header_cache_2 to read in metadata, the kmem_cache can be
    removed and the ecryptfs_header_cache_1 kmem_cache can be renamed to
    ecryptfs_header_cache.

    Signed-off-by: Tyler Hicks

    Tyler Hicks
     
  • 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
     
  • Instead of having the calling functions translate the true/false return
    code to either 0 or -EINVAL, have contains_ecryptfs_marker() return 0 or
    -EINVAL so that the calling functions can just reuse the return code.

    Also, rename the function to ecryptfs_validate_marker() to avoid callers
    mistakenly thinking that it returns true/false codes.

    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
     

26 Apr, 2011

1 commit

  • 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
     

28 Mar, 2011

2 commits

  • This patch removes the 'num_global_auth_toks' field of the
    ecryptfs_mount_crypt_stat structure, used to count the number of items in
    the 'global_auth_tok_list' list. This variable is not needed because there
    are no checks based upon it.

    Signed-off-by: Roberto Sassu
    Signed-off-by: Tyler Hicks

    Roberto Sassu
     
  • When creating a new eCryptfs file, the crypto metadata is written out
    and then the lower file was being "grown" with 4 kB of encrypted zeroes.
    I suspect that growing the encrypted file was to prevent an information
    leak that the unencrypted file was empty. However, the unencrypted file
    size is stored, in plaintext, in the metadata so growing the file is
    unnecessary.

    Signed-off-by: Tyler Hicks

    Tyler Hicks
     

18 Jan, 2011

2 commits


27 Aug, 2010

2 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
     

17 Jun, 2010

1 commit


20 Apr, 2010

1 commit

  • * '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
     

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

1 commit


23 Sep, 2009

6 commits

  • Errors returned from vfs_read() and vfs_write() calls to the lower
    filesystem were being masked as -EINVAL. This caused some confusion to
    users who saw EINVAL instead of ENOSPC when the disk was full, for
    instance.

    Also, the actual bytes read or written were not accessible by callers to
    ecryptfs_read_lower() and ecryptfs_write_lower(), which may be useful in
    some cases. This patch updates the error handling logic where those
    functions are called in order to accept positive return codes indicating
    success.

    Cc: Eric Sandeen
    Acked-by: Serge Hallyn
    Cc: ecryptfs-devel@lists.launchpad.net
    Signed-off-by: Tyler Hicks

    Tyler Hicks
     
  • Returns -ENOTSUPP when attempting to use filename encryption with
    something other than a password authentication token, such as a private
    token from openssl. Using filename encryption with a userspace eCryptfs
    key module is a future goal. Until then, this patch handles the
    situation a little better than simply using a BUG_ON().

    Acked-by: Serge Hallyn
    Cc: ecryptfs-devel@lists.launchpad.net
    Cc: stable
    Signed-off-by: Tyler Hicks

    Tyler Hicks
     
  • Returns an error when an unrecognized cipher code is present in a tag 3
    packet or an ecryptfs_crypt_stat cannot be initialized. Also sets an
    crypt_stat->tfm error pointer to NULL to ensure that it will not be
    incorrectly freed in ecryptfs_destroy_crypt_stat().

    Acked-by: Serge Hallyn
    Cc: ecryptfs-devel@lists.launchpad.net
    Cc: stable
    Signed-off-by: Tyler Hicks

    Tyler Hicks
     
  • So, I compiled a 2.6.31-rc5 kernel with ecryptfs and loaded its module.
    When it came time to mount my filesystem, I got this in dmesg, and it
    refused to mount:

    [93577.776637] Unable to allocate crypto cipher with name [aes]; rc = [-2]
    [93577.783280] Error attempting to initialize key TFM cipher with name = [aes]; rc = [-2]
    [93577.791183] Error attempting to initialize cipher with name = [aes] and key size = [32]; rc = [-2]
    [93577.800113] Error parsing options; rc = [-22]

    I figured from the error message that I'd either forgotten to load "aes"
    or that my key size was bogus. Neither one of those was the case. In
    fact, I was missing the CRYPTO_ECB config option and the 'ecb' module.
    Unfortunately, there's no trace of 'ecb' in that error message.

    I've done two things to fix this. First, I've modified ecryptfs's
    Kconfig entry to select CRYPTO_ECB and CRYPTO_CBC. I also took CRYPTO
    out of the dependencies since the 'select' will take care of it for us.

    I've also modified the error messages to print a string that should
    contain both 'ecb' and 'aes' in my error case. That will give any
    future users a chance of finding the right modules and Kconfig options.

    I also wonder if we should:

    select CRYPTO_AES if !EMBEDDED

    since I think most ecryptfs users are using AES like me.

    Cc: ecryptfs-devel@lists.launchpad.net
    Cc: linux-fsdevel@vger.kernel.org
    Cc: linux-kernel@vger.kernel.org
    Cc: Dustin Kirkland
    Signed-off-by: Dave Hansen
    [tyhicks@linux.vnet.ibm.com: Removed extra newline, 80-char violation]
    Signed-off-by: Tyler Hicks

    Dave Hansen
     
  • Lockdep reports the following valid-looking possible AB-BA deadlock with
    global_auth_tok_list_mutex and keysig_list_mutex:

    ecryptfs_new_file_context() ->
    ecryptfs_copy_mount_wide_sigs_to_inode_sigs() ->
    mutex_lock(&mount_crypt_stat->global_auth_tok_list_mutex);
    -> ecryptfs_add_keysig() ->
    mutex_lock(&crypt_stat->keysig_list_mutex);

    vs

    ecryptfs_generate_key_packet_set() ->
    mutex_lock(&crypt_stat->keysig_list_mutex);
    -> ecryptfs_find_global_auth_tok_for_sig() ->
    mutex_lock(&mount_crypt_stat->global_auth_tok_list_mutex);

    ie the two mutexes are taken in opposite orders in the two different
    code paths. I'm not sure if this is a real bug where two threads could
    actually hit the two paths in parallel and deadlock, but it at least
    makes lockdep impossible to use with ecryptfs since this report triggers
    every time and disables future lockdep reporting.

    Since ecryptfs_add_keysig() is called only from the single callsite in
    ecryptfs_copy_mount_wide_sigs_to_inode_sigs(), the simplest fix seems to
    be to move the lock of keysig_list_mutex back up outside of the where
    global_auth_tok_list_mutex is taken. This patch does that, and fixes
    the lockdep report on my system (and ecryptfs still works OK).

    The full output of lockdep fixed by this patch is:

    =======================================================
    [ INFO: possible circular locking dependency detected ]
    2.6.31-2-generic #14~rbd2
    -------------------------------------------------------
    gdm/2640 is trying to acquire lock:
    (&mount_crypt_stat->global_auth_tok_list_mutex){+.+.+.}, at: [] ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90

    but task is already holding lock:
    (&crypt_stat->keysig_list_mutex){+.+.+.}, at: [] ecryptfs_generate_key_packet_set+0x58/0x2b0

    which lock already depends on the new lock.

    the existing dependency chain (in reverse order) is:

    -> #1 (&crypt_stat->keysig_list_mutex){+.+.+.}:
    [] check_prev_add+0x2a7/0x370
    [] validate_chain+0x661/0x750
    [] __lock_acquire+0x237/0x430
    [] lock_acquire+0xa5/0x150
    [] __mutex_lock_common+0x4d/0x3d0
    [] mutex_lock_nested+0x46/0x60
    [] ecryptfs_add_keysig+0x5a/0xb0
    [] ecryptfs_copy_mount_wide_sigs_to_inode_sigs+0x59/0xb0
    [] ecryptfs_new_file_context+0xa6/0x1a0
    [] ecryptfs_initialize_file+0x4a/0x140
    [] ecryptfs_create+0x2d/0x60
    [] vfs_create+0xb4/0xe0
    [] __open_namei_create+0xc4/0x110
    [] do_filp_open+0xa01/0xae0
    [] do_sys_open+0x69/0x140
    [] sys_open+0x20/0x30
    [] system_call_fastpath+0x16/0x1b
    [] 0xffffffffffffffff

    -> #0 (&mount_crypt_stat->global_auth_tok_list_mutex){+.+.+.}:
    [] check_prev_add+0x85/0x370
    [] validate_chain+0x661/0x750
    [] __lock_acquire+0x237/0x430
    [] lock_acquire+0xa5/0x150
    [] __mutex_lock_common+0x4d/0x3d0
    [] mutex_lock_nested+0x46/0x60
    [] ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90
    [] ecryptfs_generate_key_packet_set+0x105/0x2b0
    [] ecryptfs_write_headers_virt+0xc9/0x120
    [] ecryptfs_write_metadata+0xcd/0x200
    [] ecryptfs_initialize_file+0x6b/0x140
    [] ecryptfs_create+0x2d/0x60
    [] vfs_create+0xb4/0xe0
    [] __open_namei_create+0xc4/0x110
    [] do_filp_open+0xa01/0xae0
    [] do_sys_open+0x69/0x140
    [] sys_open+0x20/0x30
    [] system_call_fastpath+0x16/0x1b
    [] 0xffffffffffffffff

    other info that might help us debug this:

    2 locks held by gdm/2640:
    #0: (&sb->s_type->i_mutex_key#11){+.+.+.}, at: [] do_filp_open+0x3cb/0xae0
    #1: (&crypt_stat->keysig_list_mutex){+.+.+.}, at: [] ecryptfs_generate_key_packet_set+0x58/0x2b0

    stack backtrace:
    Pid: 2640, comm: gdm Tainted: G C 2.6.31-2-generic #14~rbd2
    Call Trace:
    [] print_circular_bug_tail+0xa8/0xf0
    [] check_prev_add+0x85/0x370
    [] ? __module_text_address+0x12/0x60
    [] validate_chain+0x661/0x750
    [] ? print_context_stack+0x85/0x140
    [] ? find_usage_backwards+0x38/0x160
    [] __lock_acquire+0x237/0x430
    [] lock_acquire+0xa5/0x150
    [] ? ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90
    [] ? check_usage_backwards+0x0/0xb0
    [] __mutex_lock_common+0x4d/0x3d0
    [] ? ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90
    [] ? ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90
    [] ? mark_held_locks+0x6c/0xa0
    [] ? kmem_cache_alloc+0xfd/0x1a0
    [] ? trace_hardirqs_on_caller+0x14d/0x190
    [] mutex_lock_nested+0x46/0x60
    [] ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90
    [] ecryptfs_generate_key_packet_set+0x105/0x2b0
    [] ecryptfs_write_headers_virt+0xc9/0x120
    [] ecryptfs_write_metadata+0xcd/0x200
    [] ? ecryptfs_init_persistent_file+0x60/0xe0
    [] ecryptfs_initialize_file+0x6b/0x140
    [] ecryptfs_create+0x2d/0x60
    [] vfs_create+0xb4/0xe0
    [] __open_namei_create+0xc4/0x110
    [] do_filp_open+0xa01/0xae0
    [] ? _raw_spin_unlock+0x5e/0xb0
    [] ? _spin_unlock+0x2b/0x40
    [] ? getname+0x3b/0x240
    [] ? alloc_fd+0xfa/0x140
    [] do_sys_open+0x69/0x140
    [] ? trace_hardirqs_on_thunk+0x3a/0x3f
    [] sys_open+0x20/0x30
    [] system_call_fastpath+0x16/0x1b

    Signed-off-by: Roland Dreier
    Signed-off-by: Tyler Hicks

    Roland Dreier
     
  • In ecryptfs_destroy_inode(), inode_info->lower_file_mutex is locked,
    and just after the mutex is unlocked, the code does:

    kmem_cache_free(ecryptfs_inode_info_cache, inode_info);

    This means that if another context could possibly try to take the same
    mutex as ecryptfs_destroy_inode(), then it could end up getting the
    mutex just before the data structure containing the mutex is freed.
    So any such use would be an obvious use-after-free bug (catchable with
    slab poisoning or mutex debugging), and therefore the locking in
    ecryptfs_destroy_inode() is not needed and can be dropped.

    Similarly, in ecryptfs_destroy_crypt_stat(), crypt_stat->keysig_list_mutex
    is locked, and then the mutex is unlocked just before the code does:

    memset(crypt_stat, 0, sizeof(struct ecryptfs_crypt_stat));

    Therefore taking this mutex is similarly not necessary.

    Removing this locking fixes false-positive lockdep reports such as the
    following (and they are false-positives for exactly the same reason
    that the locking is not needed):

    =================================
    [ INFO: inconsistent lock state ]
    2.6.31-2-generic #14~rbd3
    ---------------------------------
    inconsistent {RECLAIM_FS-ON-W} -> {IN-RECLAIM_FS-W} usage.
    kswapd0/323 [HC0[0]:SC0[0]:HE1:SE1] takes:
    (&inode_info->lower_file_mutex){+.+.?.}, at: [] ecryptfs_destroy_inode+0x34/0x100
    {RECLAIM_FS-ON-W} state was registered at:
    [] mark_held_locks+0x6c/0xa0
    [] lockdep_trace_alloc+0xaf/0xe0
    [] kmem_cache_alloc+0x41/0x1a0
    [] get_empty_filp+0x7a/0x1a0
    [] dentry_open+0x36/0xc0
    [] ecryptfs_privileged_open+0x5c/0x2e0
    [] ecryptfs_init_persistent_file+0xa3/0xe0
    [] ecryptfs_lookup_and_interpose_lower+0x278/0x380
    [] ecryptfs_lookup+0x12a/0x250
    [] real_lookup+0xea/0x160
    [] do_lookup+0xb8/0xf0
    [] __link_path_walk+0x518/0x870
    [] path_walk+0x5c/0xc0
    [] do_path_lookup+0x5b/0xa0
    [] user_path_at+0x57/0xa0
    [] vfs_fstatat+0x3c/0x80
    [] vfs_stat+0x1b/0x20
    [] sys_newstat+0x24/0x50
    [] system_call_fastpath+0x16/0x1b
    [] 0xffffffffffffffff
    irq event stamp: 7811
    hardirqs last enabled at (7811): [] call_rcu+0x5f/0x90
    hardirqs last disabled at (7810): [] call_rcu+0x33/0x90
    softirqs last enabled at (3764): [] __do_softirq+0x14a/0x220
    softirqs last disabled at (3751): [] call_softirq+0x1c/0x30

    other info that might help us debug this:
    2 locks held by kswapd0/323:
    #0: (shrinker_rwsem){++++..}, at: [] shrink_slab+0x3d/0x190
    #1: (&type->s_umount_key#35){.+.+..}, at: [] prune_dcache+0xd1/0x1b0

    stack backtrace:
    Pid: 323, comm: kswapd0 Tainted: G C 2.6.31-2-generic #14~rbd3
    Call Trace:
    [] print_usage_bug+0x18c/0x1a0
    [] ? check_usage_forwards+0x0/0xc0
    [] mark_lock_irq+0xf2/0x280
    [] mark_lock+0x137/0x1d0
    [] ? fsnotify_clear_marks_by_inode+0x30/0xf0
    [] mark_irqflags+0xc6/0x1a0
    [] __lock_acquire+0x287/0x430
    [] lock_acquire+0xa5/0x150
    [] ? ecryptfs_destroy_inode+0x34/0x100
    [] ? __lock_acquire+0x237/0x430
    [] __mutex_lock_common+0x4d/0x3d0
    [] ? ecryptfs_destroy_inode+0x34/0x100
    [] ? fsnotify_clear_marks_by_inode+0x30/0xf0
    [] ? ecryptfs_destroy_inode+0x34/0x100
    [] ? _raw_spin_unlock+0x5e/0xb0
    [] mutex_lock_nested+0x46/0x60
    [] ecryptfs_destroy_inode+0x34/0x100
    [] destroy_inode+0x87/0xd0
    [] generic_delete_inode+0x12c/0x1a0
    [] iput+0x62/0x70
    [] dentry_iput+0x98/0x110
    [] d_kill+0x50/0x80
    [] prune_one_dentry+0xa3/0xc0
    [] __shrink_dcache_sb+0x271/0x290
    [] prune_dcache+0x109/0x1b0
    [] shrink_dcache_memory+0x3f/0x50
    [] shrink_slab+0x12d/0x190
    [] balance_pgdat+0x4d7/0x640
    [] ? finish_task_switch+0x40/0x150
    [] ? isolate_pages_global+0x0/0x60
    [] kswapd+0x117/0x170
    [] ? autoremove_wake_function+0x0/0x40
    [] ? kswapd+0x0/0x170
    [] kthread+0x9e/0xb0
    [] child_rip+0xa/0x20
    [] ? restore_args+0x0/0x30
    [] ? kthread+0x0/0xb0
    [] ? child_rip+0x0/0x20

    Signed-off-by: Roland Dreier
    Signed-off-by: Tyler Hicks

    Roland Dreier
     

22 Apr, 2009

1 commit

  • ecryptfs_passthrough is a mount option that allows eCryptfs to allow
    data to be written to non-eCryptfs files in the lower filesystem. The
    passthrough option was causing data corruption due to it not always
    being treated as a non-eCryptfs file.

    The first 8 bytes of an eCryptfs file contains the decrypted file size.
    This value was being written to the non-eCryptfs files, too. Also,
    extra 0x00 characters were being written to make the file size a
    multiple of PAGE_CACHE_SIZE.

    Signed-off-by: Tyler Hicks

    Tyler Hicks
     

23 Mar, 2009

2 commits

  • If ecryptfs_encrypted_view or ecryptfs_xattr_metadata were being
    specified as mount options, a NULL pointer dereference of crypt_stat
    was possible during lookup.

    This patch moves the crypt_stat assignment into
    ecryptfs_lookup_and_interpose_lower(), ensuring that crypt_stat
    will not be NULL before we attempt to dereference it.

    Thanks to Dan Carpenter and his static analysis tool, smatch, for
    finding this bug.

    Signed-off-by: Tyler Hicks
    Acked-by: Dustin Kirkland
    Cc: Dan Carpenter
    Cc: Serge Hallyn
    Signed-off-by: Linus Torvalds

    Tyler Hicks
     
  • When allocating the memory used to store the eCryptfs header contents, a
    single, zeroed page was being allocated with get_zeroed_page().
    However, the size of an eCryptfs header is either PAGE_CACHE_SIZE or
    ECRYPTFS_MINIMUM_HEADER_EXTENT_SIZE (8192), whichever is larger, and is
    stored in the file's private_data->crypt_stat->num_header_bytes_at_front
    field.

    ecryptfs_write_metadata_to_contents() was using
    num_header_bytes_at_front to decide how many bytes should be written to
    the lower filesystem for the file header. Unfortunately, at least 8K
    was being written from the page, despite the chance of the single,
    zeroed page being smaller than 8K. This resulted in random areas of
    kernel memory being written between the 0x1000 and 0x1FFF bytes offsets
    in the eCryptfs file headers if PAGE_SIZE was 4K.

    This patch allocates a variable number of pages, calculated with
    num_header_bytes_at_front, and passes the number of allocated pages
    along to ecryptfs_write_metadata_to_contents().

    Thanks to Florian Streibelt for reporting the data leak and working with
    me to find the problem. 2.6.28 is the only kernel release with this
    vulnerability. Corresponds to CVE-2009-0787

    Signed-off-by: Tyler Hicks
    Acked-by: Dustin Kirkland
    Reviewed-by: Eric Sandeen
    Reviewed-by: Eugene Teo
    Cc: Greg KH
    Cc: dann frazier
    Cc: Serge E. Hallyn
    Cc: Florian Streibelt
    Cc: stable@kernel.org
    Signed-off-by: Linus Torvalds

    Tyler Hicks
     

15 Mar, 2009

1 commit

  • eCryptfs has file encryption keys (FEK), file encryption key encryption
    keys (FEKEK), and filename encryption keys (FNEK). The per-file FEK is
    encrypted with one or more FEKEKs and stored in the header of the
    encrypted file. I noticed that the FEK is also being encrypted by the
    FNEK. This is a problem if a user wants to use a different FNEK than
    their FEKEK, as their file contents will still be accessible with the
    FNEK.

    This is a minimalistic patch which prevents the FNEKs signatures from
    being copied to the inode signatures list. Ultimately, it keeps the FEK
    from being encrypted with a FNEK.

    Signed-off-by: Tyler Hicks
    Cc: Serge Hallyn
    Acked-by: Dustin Kirkland
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Tyler Hicks
     

07 Feb, 2009

1 commit

  • The addition of filename encryption caused a regression in unencrypted
    filename symlink support. ecryptfs_copy_filename() is used when dealing
    with unencrypted filenames and it reported that the new, copied filename
    was a character longer than it should have been.

    This caused the return value of readlink() to count the NULL byte of the
    symlink target. Most applications don't care about the extra NULL byte,
    but a version control system (bzr) helped in discovering the bug.

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

    Tyler Hicks
     

07 Jan, 2009

7 commits

  • Flesh out the comments for ecryptfs_decode_from_filename(). Remove the
    return condition, since it is always 0.

    Signed-off-by: Michael Halcrow
    Cc: Dustin Kirkland
    Cc: Eric Sandeen
    Cc: Tyler Hicks
    Cc: David Kleikamp
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Michael Halcrow
     
  • Correct several format string data type specifiers. Correct filename size
    data types; they should be size_t rather than int when passed as
    parameters to some other functions (although note that the filenames will
    never be larger than int).

    Signed-off-by: Michael Halcrow
    Cc: Dustin Kirkland
    Cc: Eric Sandeen
    Cc: Tyler Hicks
    Cc: David Kleikamp
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Michael Halcrow
     
  • %Z is a gcc-ism. Using %z instead.

    Signed-off-by: Michael Halcrow
    Cc: Dustin Kirkland
    Cc: Eric Sandeen
    Cc: Tyler Hicks
    Cc: David Kleikamp
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Michael Halcrow
     
  • Make the requisite modifications to ecryptfs_filldir(), ecryptfs_lookup(),
    and ecryptfs_readlink() to call out to filename encryption functions.
    Propagate filename encryption policy flags from mount-wide crypt_stat to
    inode crypt_stat.

    Signed-off-by: Michael Halcrow
    Cc: Dustin Kirkland
    Cc: Eric Sandeen
    Cc: Tyler Hicks
    Cc: David Kleikamp
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Michael Halcrow
     
  • These functions support encrypting and encoding the filename contents.
    The encrypted filename contents may consist of any ASCII characters. This
    patch includes a custom encoding mechanism to map the ASCII characters to
    a reduced character set that is appropriate for filenames.

    Signed-off-by: Michael Halcrow
    Cc: Dustin Kirkland
    Cc: Eric Sandeen
    Cc: Tyler Hicks
    Cc: David Kleikamp
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Michael Halcrow
     
  • Extensions to the header file to support filename encryption.

    Signed-off-by: Michael Halcrow
    Cc: Dustin Kirkland
    Cc: Eric Sandeen
    Cc: Tyler Hicks
    Cc: David Kleikamp
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Michael Halcrow
     
  • This patchset implements filename encryption via a passphrase-derived
    mount-wide Filename Encryption Key (FNEK) specified as a mount parameter.
    Each encrypted filename has a fixed prefix indicating that eCryptfs should
    try to decrypt the filename. When eCryptfs encounters this prefix, it
    decodes the filename into a tag 70 packet and then decrypts the packet
    contents using the FNEK, setting the filename to the decrypted filename.
    Both unencrypted and encrypted filenames can reside in the same lower
    filesystem.

    Because filename encryption expands the length of the filename during the
    encoding stage, eCryptfs will not properly handle filenames that are
    already near the maximum filename length.

    In the present implementation, eCryptfs must be able to produce a match
    against the lower encrypted and encoded filename representation when given
    a plaintext filename. Therefore, two files having the same plaintext name
    will encrypt and encode into the same lower filename if they are both
    encrypted using the same FNEK. This can be changed by finding a way to
    replace the prepended bytes in the blocked-aligned filename with random
    characters; they are hashes of the FNEK right now, so that it is possible
    to deterministically map from a plaintext filename to an encrypted and
    encoded filename in the lower filesystem. An implementation using random
    characters will have to decode and decrypt every single directory entry in
    any given directory any time an event occurs wherein the VFS needs to
    determine whether a particular file exists in the lower directory and the
    decrypted and decoded filenames have not yet been extracted for that
    directory.

    Thanks to Tyler Hicks and David Kleikamp for assistance in the development
    of this patchset.

    This patch:

    A tag 70 packet contains a filename encrypted with a Filename Encryption
    Key (FNEK). This patch implements functions for writing and parsing tag
    70 packets. This patch also adds definitions and extends structures to
    support filename encryption.

    Signed-off-by: Michael Halcrow
    Cc: Dustin Kirkland
    Cc: Eric Sandeen
    Cc: Tyler Hicks
    Cc: David Kleikamp
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Michael Halcrow
     

31 Oct, 2008

1 commit

  • When ecryptfs allocates space to write crypto headers into, before copying
    it out to file headers or to xattrs, it looks at the value of
    crypt_stat->num_header_bytes_at_front to determine how much space it
    needs. This is also used as the file offset to the actual encrypted data,
    so for xattr-stored crypto info, the value was zero.

    So, we kzalloc'd 0 bytes, and then ran off to write to that memory.
    (Which returned as ZERO_SIZE_PTR, so we explode quickly).

    The right answer is to always allocate a page to write into; the current
    code won't ever write more than that (this is enforced by the
    (PAGE_CACHE_SIZE - offset) length in the call to
    ecryptfs_generate_key_packet_set). To be explicit about this, we now send
    in a "max" parameter, rather than magically using PAGE_CACHE_SIZE there.

    Also, since the pointer we pass down the callchain eventually gets the
    virt_to_page() treatment, we should be using a alloc_page variant, not
    kzalloc (see also 7fcba054373d5dfc43d26e243a5c9b92069972ee)

    Signed-off-by: Eric Sandeen
    Acked-by: Michael Halcrow
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Eric Sandeen