From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dustin Kirkland Subject: Re: [PATCH] eCryptfs: Use notify_change for truncating lower inodes Date: Thu, 15 Oct 2009 11:55:23 -0500 Message-ID: <1255625723.12108.42.camel@x200> References: <4AD5F036.5030209@linux.vnet.ibm.com> <1255580377-16577-1-git-send-email-tyhicks@linux.vnet.ibm.com> Reply-To: kirkland@canonical.com Mime-Version: 1.0 Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-r5WD44NOFDJ5cM2i/Iyy" Cc: linux-fsdevel@vger.kernel.org, Christoph Hellwig , ecryptfs-devel@lists.launchpad.net To: Tyler Hicks Return-path: Received: from adelie.canonical.com ([91.189.90.139]:59536 "EHLO adelie.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935148AbZJOQ4H (ORCPT ); Thu, 15 Oct 2009 12:56:07 -0400 In-Reply-To: <1255580377-16577-1-git-send-email-tyhicks@linux.vnet.ibm.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: --=-r5WD44NOFDJ5cM2i/Iyy Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2009-10-14 at 23:19 -0500, Tyler Hicks wrote: > 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(). >=20 > This patch moves the lower inode truncation out of ecryptfs_truncate() > and renames the function 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. >=20 > For eCryptfs functions needing to truncate, ecryptfs_truncate() is > reintroduced as a simple way to truncate the upper inode to a specified > size and then truncate the lower inode accordingly. >=20 > https://bugs.launchpad.net/bugs/451368 >=20 > Reported-by: Christoph Hellwig > Cc: Dustin Kirkland > Cc: ecryptfs-devel@lists.launchpad.net > Cc: linux-fsdevel@vger.kernel.org > Signed-off-by: Tyler Hicks Thanks for tackling, this, Tyler. Looks good to me. Acked-by: Dustin Kirkland > --- > fs/ecryptfs/inode.c | 97 ++++++++++++++++++++++++++++++++++-----------= ------ > 1 files changed, 65 insertions(+), 32 deletions(-) >=20 > diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c > index 056fed6..371e92e 100644 > --- a/fs/ecryptfs/inode.c > +++ b/fs/ecryptfs/inode.c > @@ -772,18 +772,23 @@ upper_size_to_lower_size(struct ecryptfs_crypt_stat= *crypt_stat, > } > =20 > /** > - * ecryptfs_truncate > + * truncate_upper > * @dentry: The ecryptfs layer dentry > - * @new_length: The length to expand the file to > + * @ia: Address of the ecryptfs inode's attributes > + * @lower_ia: Address of the lower inode's attributes > * > * Function to handle truncations modifying the size of the file. Note > * that the file sizes are interpolated. When expanding, we are simply > - * writing strings of 0's out. When truncating, we need to modify the > - * underlying file size according to the page index interpolations. > + * writing strings of 0's out. When truncating, we truncate the upper > + * inode and update the lower_ia according to the page index > + * interpolations. If ATTR_SIZE is set in lower_ia->ia_valid upon return= , > + * the caller must use lower_ia in a call to notify_change() to perform > + * the truncation of the lower inode. > * > * Returns zero on success; non-zero otherwise > */ > -int ecryptfs_truncate(struct dentry *dentry, loff_t new_length) > +static int truncate_upper(struct dentry *dentry, struct iattr *ia, > + struct iattr *lower_ia) > { > int rc =3D 0; > struct inode *inode =3D dentry->d_inode; > @@ -794,7 +799,7 @@ int ecryptfs_truncate(struct dentry *dentry, loff_t n= ew_length) > loff_t lower_size_before_truncate; > loff_t lower_size_after_truncate; > =20 > - if (unlikely((new_length =3D=3D i_size))) > + if (unlikely((ia->ia_size =3D=3D i_size))) > goto out; > crypt_stat =3D &ecryptfs_inode_to_private(dentry->d_inode)->crypt_stat; > /* Set up a fake ecryptfs file, this is used to interface with > @@ -815,28 +820,30 @@ int ecryptfs_truncate(struct dentry *dentry, loff_t= new_length) > &fake_ecryptfs_file, > ecryptfs_inode_to_private(dentry->d_inode)->lower_file); > /* Switch on growing or shrinking file */ > - if (new_length > i_size) { > + if (ia->ia_size > i_size) { > char zero[] =3D { 0x00 }; > =20 > + lower_ia->ia_valid &=3D ~ATTR_SIZE; > /* Write a single 0 at the last position of the file; > * this triggers code that will fill in 0's throughout > * the intermediate portion of the previous end of the > * file and the new and of the file */ > rc =3D ecryptfs_write(&fake_ecryptfs_file, zero, > - (new_length - 1), 1); > - } else { /* new_length < i_size_read(inode) */ > - /* We're chopping off all the pages down do the page > - * in which new_length is located. Fill in the end of > - * that page from (new_length & ~PAGE_CACHE_MASK) to > + (ia->ia_size - 1), 1); > + } else { /* ia->ia_size < i_size_read(inode) */ > + /* We're chopping off all the pages down to the page > + * in which ia->ia_size is located. Fill in the end of > + * that page from (ia->ia_size & ~PAGE_CACHE_MASK) to > * PAGE_CACHE_SIZE with zeros. */ > size_t num_zeros =3D (PAGE_CACHE_SIZE > - - (new_length & ~PAGE_CACHE_MASK)); > + - (ia->ia_size & ~PAGE_CACHE_MASK)); > =20 > if (!(crypt_stat->flags & ECRYPTFS_ENCRYPTED)) { > - rc =3D vmtruncate(inode, new_length); > + rc =3D vmtruncate(inode, ia->ia_size); > if (rc) > goto out_free; > - rc =3D vmtruncate(lower_dentry->d_inode, new_length); > + lower_ia->ia_size =3D ia->ia_size; > + lower_ia->ia_valid |=3D ATTR_SIZE; > goto out_free; > } > if (num_zeros) { > @@ -848,7 +855,7 @@ int ecryptfs_truncate(struct dentry *dentry, loff_t n= ew_length) > goto out_free; > } > rc =3D ecryptfs_write(&fake_ecryptfs_file, zeros_virt, > - new_length, num_zeros); > + ia->ia_size, num_zeros); > kfree(zeros_virt); > if (rc) { > printk(KERN_ERR "Error attempting to zero out " > @@ -857,7 +864,7 @@ int ecryptfs_truncate(struct dentry *dentry, loff_t n= ew_length) > goto out_free; > } > } > - vmtruncate(inode, new_length); > + vmtruncate(inode, ia->ia_size); > rc =3D ecryptfs_write_inode_size_to_metadata(inode); > if (rc) { > printk(KERN_ERR "Problem with " > @@ -870,10 +877,12 @@ int ecryptfs_truncate(struct dentry *dentry, loff_t= new_length) > lower_size_before_truncate =3D > upper_size_to_lower_size(crypt_stat, i_size); > lower_size_after_truncate =3D > - upper_size_to_lower_size(crypt_stat, new_length); > - if (lower_size_after_truncate < lower_size_before_truncate) > - vmtruncate(lower_dentry->d_inode, > - lower_size_after_truncate); > + upper_size_to_lower_size(crypt_stat, ia->ia_size); > + if (lower_size_after_truncate < lower_size_before_truncate) { > + lower_ia->ia_size =3D lower_size_after_truncate; > + lower_ia->ia_valid |=3D ATTR_SIZE; > + } else > + lower_ia->ia_valid &=3D ~ATTR_SIZE; > } > out_free: > if (ecryptfs_file_to_private(&fake_ecryptfs_file)) > @@ -883,6 +892,33 @@ out: > return rc; > } > =20 > +/** > + * ecryptfs_truncate > + * @dentry: The ecryptfs layer dentry > + * @new_length: The length to expand the file to > + * > + * Simple function that handles the truncation of an eCryptfs inode and > + * its corresponding lower inode. > + * > + * Returns zero on success; non-zero otherwise > + */ > +int ecryptfs_truncate(struct dentry *dentry, loff_t new_length) > +{ > + struct iattr ia =3D { .ia_valid =3D ATTR_SIZE, .ia_size =3D new_length = }; > + struct iattr lower_ia =3D { .ia_valid =3D 0 }; > + int rc; > + > + rc =3D truncate_upper(dentry, &ia, &lower_ia); > + if (!rc && lower_ia.ia_valid & ATTR_SIZE) { > + struct dentry *lower_dentry =3D ecryptfs_dentry_to_lower(dentry); > + > + mutex_lock(&lower_dentry->d_inode->i_mutex); > + rc =3D notify_change(lower_dentry, &lower_ia); > + mutex_unlock(&lower_dentry->d_inode->i_mutex); > + } > + return rc; > +} > + > static int > ecryptfs_permission(struct inode *inode, int mask) > { > @@ -905,6 +941,7 @@ static int ecryptfs_setattr(struct dentry *dentry, st= ruct iattr *ia) > { > int rc =3D 0; > struct dentry *lower_dentry; > + struct iattr lower_ia; > struct inode *inode; > struct inode *lower_inode; > struct ecryptfs_crypt_stat *crypt_stat; > @@ -943,15 +980,11 @@ static int ecryptfs_setattr(struct dentry *dentry, = struct iattr *ia) > } > } > mutex_unlock(&crypt_stat->cs_mutex); > + memcpy(&lower_ia, ia, sizeof(lower_ia)); > + if (ia->ia_valid & ATTR_FILE) > + lower_ia.ia_file =3D ecryptfs_file_to_lower(ia->ia_file); > if (ia->ia_valid & ATTR_SIZE) { > - ecryptfs_printk(KERN_DEBUG, > - "ia->ia_valid =3D [0x%x] ATTR_SIZE" " =3D [0x%x]\n", > - ia->ia_valid, ATTR_SIZE); > - rc =3D ecryptfs_truncate(dentry, ia->ia_size); > - /* ecryptfs_truncate handles resizing of the lower file */ > - ia->ia_valid &=3D ~ATTR_SIZE; > - ecryptfs_printk(KERN_DEBUG, "ia->ia_valid =3D [%x]\n", > - ia->ia_valid); > + rc =3D truncate_upper(dentry, ia, &lower_ia); > if (rc < 0) > goto out; > } > @@ -960,11 +993,11 @@ static int ecryptfs_setattr(struct dentry *dentry, = struct iattr *ia) > * mode change is for clearing setuid/setgid bits. Allow lower fs > * to interpret this in its own way. > */ > - if (ia->ia_valid & (ATTR_KILL_SUID | ATTR_KILL_SGID)) > - ia->ia_valid &=3D ~ATTR_MODE; > + if (lower_ia.ia_valid & (ATTR_KILL_SUID | ATTR_KILL_SGID)) > + lower_ia.ia_valid &=3D ~ATTR_MODE; > =20 > mutex_lock(&lower_dentry->d_inode->i_mutex); > - rc =3D notify_change(lower_dentry, ia); > + rc =3D notify_change(lower_dentry, &lower_ia); > mutex_unlock(&lower_dentry->d_inode->i_mutex); > out: > fsstack_copy_attr_all(inode, lower_inode, NULL); --=-r5WD44NOFDJ5cM2i/Iyy Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) iEYEABECAAYFAkrXU/sACgkQs7pNXIOmEZSoZwCfW+nnVh33HkIsbHuDDgX/sZCn t5AAoL+cfUhyDIe5nLJvLak2CbZSlkU2 =CjLR -----END PGP SIGNATURE----- --=-r5WD44NOFDJ5cM2i/Iyy--