linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrey Pronin <apronin@chromium.org>
To: tyhicks@canonical.com
Cc: ecryptfs@vger.kernel.org, linux-kernel@vger.kernel.org,
	gwendal@chromium.org, dtor@chromium.org,
	Andrey Pronin <apronin@chromium.org>
Subject: [PATCH] CHROMIUM: ecryptfs: sync before truncating lower inode
Date: Tue, 18 Apr 2017 16:36:49 -0700	[thread overview]
Message-ID: <20170418233649.78805-1-apronin@chromium.org> (raw)

If the updated ecryptfs header data is not written to disk before
the lower file is truncated, a crash may leave the filesystem
in the state when the lower file truncation is journaled, while
the changes to the ecryptfs header are lost (if the underlying
filesystem is ext4 in data=ordered mode, for example). As a result,
upon remounting and repairing the file may have a pre-truncation
length and garbage data after the post-truncation end.

To reproduce, make a snapshot of the underlying ext4 filesystem
mounted with data=ordered while asynchronously truncating to zero a
group of files in ecryptfs mounted on top. Mount ecryptfs for the
snapshot and check the contents of the group of files that was
being truncated. The following script reproduces it in almost 100%
of runs:

cd /tmp
mkdir -p ./loop
dd if=/dev/zero of=./file.img bs=1M count=10
PW=secret

LOOPDEV=`losetup --find --show ./file.img`
mkfs -t ext4 $LOOPDEV
mount -t ext4 -o rw,nodev,relatime,seclabel,commit=600,data=ordered\
 $LOOPDEV ./loop
mkdir -p ./loop/vault ./loop/mount
mount -t ecryptfs -o rw,relatime,seclabel,ecryptfs_cipher=aes,\
ecryptfs_key_bytes=16,ecryptfs_unlink_sigs,ecryptfs_passthrough=no,\
ecryptfs_enable_filename_crypto=no,passphrase_passwd="$PW",no_sig_cache\
 ./loop/vault ./loop/mount
for i in `seq 1 100`; do echo $i > ./loop/mount/test.$i; done
sync
for i in `seq 100 -1 1`; do truncate -s 0 ./loop/mount/test.$i; done &
sleep 0.1; sync; cp ./file.img ./file.snap; sleep 1
umount ./loop/mount
umount ./loop
losetup -d $LOOPDEV

LOOPDEV=`losetup --find --show ./file.snap`
mount -t ext4 -o rw,nodev,relatime,seclabel,commit=600,data=ordered\
 $LOOPDEV ./loop
mount -t ecryptfs -o rw,relatime,seclabel,ecryptfs_cipher=aes,\
ecryptfs_key_bytes=16,ecryptfs_unlink_sigs,ecryptfs_passthrough=no,\
ecryptfs_enable_filename_crypto=no,passphrase_passwd="$PW",no_sig_cache\
 ./loop/vault ./loop/mount
for i in `seq 1 100`; do
  if [ `stat -c %s ./loop/mount/test.$i` != 0 ] &&
     [ `cat ./loop/mount/test.$i` != $i ]; then
       echo -n "!!! garbage at $i: ";  cat ./loop/mount/test.$i; echo
  fi
done
umount ./loop/mount
umount ./loop
losetup -d $LOOPDEV

Signed-off-by: Andrey Pronin <apronin@chromium.org>
---
 fs/ecryptfs/ecryptfs_kernel.h |  1 +
 fs/ecryptfs/inode.c           |  6 ++++++
 fs/ecryptfs/read_write.c      | 22 ++++++++++++++++++++++
 3 files changed, 29 insertions(+)

diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index f622a733f7ad..567698421270 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -689,6 +689,7 @@ int ecryptfs_read_lower_page_segment(struct page *page_for_ecryptfs,
 				     pgoff_t page_index,
 				     size_t offset_in_page, size_t size,
 				     struct inode *ecryptfs_inode);
+int ecryptfs_fsync_lower(struct inode *ecryptfs_inode, int datasync);
 struct page *ecryptfs_get_locked_page(struct inode *inode, loff_t index);
 int ecryptfs_parse_packet_length(unsigned char *data, size_t *size,
 				 size_t *length_size);
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 5eab400e2590..e7eb8ea154d2 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -827,6 +827,12 @@ static int truncate_upper(struct dentry *dentry, struct iattr *ia,
 			       "rc = [%d]\n", rc);
 			goto out;
 		}
+		rc = ecryptfs_fsync_lower(inode, 0);
+		if (rc) {
+			printk(KERN_WARNING "Problem with ecryptfs_fsync_lower,"
+			       "continue without syncing; "
+			       "rc = [%d]\n", rc);
+		}
 		/* We are reducing the size of the ecryptfs file, and need to
 		 * know if we need to reduce the size of the lower file. */
 		lower_size_before_truncate =
diff --git a/fs/ecryptfs/read_write.c b/fs/ecryptfs/read_write.c
index 09fe622274e4..ba2dd6263875 100644
--- a/fs/ecryptfs/read_write.c
+++ b/fs/ecryptfs/read_write.c
@@ -271,3 +271,25 @@ int ecryptfs_read_lower_page_segment(struct page *page_for_ecryptfs,
 	flush_dcache_page(page_for_ecryptfs);
 	return rc;
 }
+
+/**
+ * ecryptfs_fsync_lower
+ * @ecryptfs_inode: The eCryptfs inode
+ * @datasync: Only perform a fdatasync operation
+ *
+ * Write back data and metadata for the lower file to disk.  If @datasync is
+ * set only metadata needed to access modified file data is written.
+ *
+ * Returns 0 on success; less than zero on error
+ */
+int ecryptfs_fsync_lower(struct inode *ecryptfs_inode, int datasync)
+{
+	struct file *lower_file;
+
+	lower_file = ecryptfs_inode_to_private(ecryptfs_inode)->lower_file;
+	if (!lower_file)
+		return -EIO;
+	if (!lower_file->f_op->fsync)
+		return 0;
+	return vfs_fsync(lower_file, datasync);
+}
-- 
2.12.2.816.g2cccc81164-goog

             reply	other threads:[~2017-04-18 23:36 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-18 23:36 Andrey Pronin [this message]
2017-04-20 23:27 ` [PATCH] CHROMIUM: ecryptfs: sync before truncating lower inode Tyler Hicks
2017-04-21 23:52   ` Andrey Pronin
2017-04-26 17:43     ` Andrey Pronin
2017-04-26 18:02 ` [PATCH v2] " Andrey Pronin
2017-11-16  0:58   ` Dmitry Torokhov
2017-11-18  0:49     ` Tyler Hicks

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170418233649.78805-1-apronin@chromium.org \
    --to=apronin@chromium.org \
    --cc=dtor@chromium.org \
    --cc=ecryptfs@vger.kernel.org \
    --cc=gwendal@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tyhicks@canonical.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).