ecryptfs_truncate should not call vmtruncate on lower inode

Bug #451368 reported by Tyler Hicks
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
eCryptfs
Fix Released
High
Tyler Hicks
linux (Ubuntu)
Fix Released
Undecided
Unassigned
Lucid
Fix Released
Undecided
Colin Ian King

Bug Description

From Christoph Hellwig:

"It looks like ecryptfs calls vmtruncate directly on the lower filesystem, which is quite wrong. Vmtruncate is only a helper for the filesystem, and while most filesystems end up calling vmtruncate from their ->setattr implementation if ATTR_SIZE is set there are many that require additional work. I think ecryptfs needs to got through notify_change() / ->setattr if it wants truncate to work reliably and without silent corruption or leaking blocks on a variety of filesystems."

See entire thread here:

http://thread.gmane.org/gmane.linux.file-systems/36105

Tyler Hicks (tyhicks)
Changed in ecryptfs:
status: Triaged → In Progress
Revision history for this message
Tyler Hicks (tyhicks) wrote :

Patch submitted for review:
http://article.gmane.org/gmane.linux.file-systems/36107

Has been applied to:
git://git.kernel.org/pub/scm/linux/kernel/git/ecryptfs/ecryptfs-2.6.git#next

Changed in ecryptfs:
status: In Progress → Fix Committed
Revision history for this message
Tyler Hicks (tyhicks) wrote :
Changed in ecryptfs:
status: Fix Committed → Fix Released
Revision history for this message
Colin Ian King (colin-king) wrote :

SRU Justification:

When truncating inodes in the lower filesystem, eCryptfs directly
invoked vmtruncate(). As Christoph Hellwig pointed out, vmtruncate() is
a filesystem helper function, but filesystems may need to do more than
just a call to vmtruncate(). eCryptfs needs to go through
notify_change()->setattr to ensure truncate works reliable without
corruption or leaking blocks on a variety of filesystems.

See: http://thread.gmane.org/gmane.linux.file-systems/36105

Fix: With this patch , lower inode truncation is moved out of
ecryptfs_truncate() and the function is renamed to truncate_upper().
truncate_upper() updates an iattr for the lower inode to indicate
if the lower inode needs to be truncated upon return.
ecryptfs_setattr() then calls notify_change(), using the updated iattr
for the lower inode, to complete the truncation.

Without the patch we risk corrupting or leaking blocks on the lower
filesystem.

Has been soak tested with a variety of ftruncate() soak tests and also
running a full kernel build on ecryptfs with a lower ext4 filesystem.

Revision history for this message
Brad Figg (brad-figg) wrote : Missing required logs.

This bug is missing log files that will aid in diagnosing the problem. From a terminal window please run:

apport-collect 451368

and then change the status of the bug to 'Confirmed'.

If, due to the nature of the issue you have encountered, you are unable to run this command, please add a comment stating that fact and change the bug status to 'Confirmed'.

This change has been made by an automated script, maintained by the Ubuntu Kernel Team.

Changed in linux (Ubuntu):
status: New → Incomplete
Revision history for this message
Colin Ian King (colin-king) wrote :

Attached is a truncate soak tester.

Tim Gardner (timg-tpi)
Changed in linux (Ubuntu Lucid):
status: New → Fix Committed
Revision history for this message
Herton R. Krzesinski (herton) wrote :

The commit for this issue in Lucid is an early application of a commit that will be coming in via upstream stable (2.6.32.58). As such it is not subject to the standard bug verification process.

tags: added: verification-done-lucid
Changed in linux (Ubuntu Lucid):
assignee: nobody → Colin King (colin-king)
Changed in linux (Ubuntu):
status: Incomplete → Fix Released
Revision history for this message
Colin Ian King (colin-king) wrote :

Tested and verified working for Lucid -proposed i386 2.6.32-40.87

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package linux - 2.6.32-40.87

---------------
linux (2.6.32-40.87) lucid-proposed; urgency=low

  [Herton R. Krzesinski]

  * Release Tracking Bug
    - LP: #947375

  [ Upstream Kernel Changes ]

  * IB/mlx4: pass SMP vendor-specific attribute MADs to firmware
    - LP: #932043
  * mm/filemap_xip.c: fix race condition in xip_file_fault()
    - LP: #932043
  * NFSv4: Fix up the callers of nfs4_state_end_reclaim_reboot
    - LP: #932043
  * NFSv4: The state manager shouldn't exit on errors that were handled
    - LP: #932043
  * NFSv4: Ensure the state manager handles NFS4ERR_NO_GRACE correctly
    - LP: #932043
  * NFSv4: Handle NFS4ERR_GRACE when recovering an expired lease.
    - LP: #932043
  * NFSv4: Fix open recovery
    - LP: #932043
  * rpc client can not deal with ENOSOCK, so translate it into ENOCONN
    - LP: #932043
  * udf: Mark LVID buffer as uptodate before marking it dirty
    - LP: #932043
  * eCryptfs: Infinite loop due to overflow in ecryptfs_write()
    - LP: #932043
  * atmel_lcdfb: fix usage of CONTRAST_CTR in suspend/resume
    - LP: #932043
  * Staging: asus_oled: fix image processing
    - LP: #932043
  * Staging: android: binder: Don't call dump_stack in binder_vma_open
    - LP: #932043
  * Staging: android: binder: Fix crashes when sharing a binder file
    between processes
    - LP: #932043
  * usb: gadget: zero: fix bug in loopback autoresume handling
    - LP: #932043
  * usb: Skip PCI USB quirk handling for Netlogic XLP
    - LP: #932043
  * USB: usbserial: add new PID number (0xa951) to the ftdi driver
    - LP: #932043
  * mmc: cb710 core: Add missing spin_lock_init for irq_lock of struct
    cb710_chip
    - LP: #932043
  * net: fix sk_forward_alloc corruptions
    - LP: #932043
  * net: sock_queue_err_skb() dont mess with sk_forward_alloc
    - LP: #932043
  * Linux 2.6.32.57
    - LP: #932043
  * Ban ecryptfs over ecryptfs
    - LP: #932987
  * eCryptfs: Remove mmap from directory operations
    - LP: #400443
  * eCryptfs: Use notify_change for truncating lower inodes
    - LP: #451368
  * ecryptfs: read on a directory should return EISDIR if not supported
    - LP: #719691
  * eCryptfs: Remove extra d_delete in ecryptfs_rmdir
    - LP: #723518
  * eCryptfs: Clear i_nlink in rmdir
    - LP: #723518
  * KVM: Device assignment permission checks
    - LP: #897812
    - CVE-2011-4347
  * block: Fix io_context leak after clone with CLONE_IO
    - LP: #940743
    - CVE-2012-0879
  * block: Fix io_context leak after failure of clone with CLONE_IO
    - LP: #940743
    - CVE-2012-0879
  * eCryptfs: Handle failed metadata read in lookup
    - LP: #509180
  * drm/i915: Fix TV Out refresh rate.
    - LP: #945114
  * Linux 2.6.32.57+drm33.23
    - LP: #945114
 -- Herton Ronaldo Krzesinski <email address hidden> Mon, 05 Mar 2012 16:09:18 -0300

Changed in linux (Ubuntu Lucid):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.