From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tyler Hicks Subject: Re: [PATCH] ecryptfs: don't ignore return value from lock_rename Date: Mon, 7 Dec 2009 15:46:06 -0600 Message-ID: <20091207214606.GA9807@fedora-virt> References: <200912060217.nB62H945020961@agora.fsl.cs.sunysb.edu> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Dustin Kirkland , Andrew Morton , viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org To: Erez Zadok Return-path: Received: from e33.co.us.ibm.com ([32.97.110.151]:59066 "EHLO e33.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965066AbZLGVqH (ORCPT ); Mon, 7 Dec 2009 16:46:07 -0500 Received: from d03relay03.boulder.ibm.com (d03relay03.boulder.ibm.com [9.17.195.228]) by e33.co.us.ibm.com (8.14.3/8.13.1) with ESMTP id nB7LhKpD009778 for ; Mon, 7 Dec 2009 14:43:20 -0700 Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d03relay03.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id nB7Lk7BR101130 for ; Mon, 7 Dec 2009 14:46:08 -0700 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.14.3/8.13.1/NCO v10.0 AVout) with ESMTP id nB7Lk66b027360 for ; Mon, 7 Dec 2009 14:46:07 -0700 Content-Disposition: inline In-Reply-To: <200912060217.nB62H945020961@agora.fsl.cs.sunysb.edu> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Sat Dec 05, 2009 at 09:17:09PM -0500, Erez Zadok (ezk@cs.sunysb.edu) was quoted: > Tyler/Dustin, > > Here's another patch: use return value from lock_rename() in > ecryptfs_rename, and properly check that return value. I've tested it > briefly with ecryptfs: moved some files/directories at both the same level, > as well as into subdirectories which may share a common ancestor. Things > seem to work, but perhaps more testing is needed. > > BTW, identical code to this has been running in Unionfs for a couple of > years, so I'm reasonably confident that this patch is correct. > > diffstat: > inode.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > Cheers, > Erez. > > ------------------------------------------------------------------------------ > > ecryptfs: don't ignore return value from lock_rename > > Signed-off-by: Erez Zadok Looks correct to me - thanks! Applied to git://git.kernel.org/pub/scm/linux/kernel/git/ecryptfs/ecryptfs-2.6.git#next > diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c > index 429ca0b..e4dd0c6 100644 > --- a/fs/ecryptfs/inode.c > +++ b/fs/ecryptfs/inode.c > @@ -614,6 +614,7 @@ ecryptfs_rename(struct inode *old_dir, struct dentry *old_dentry, > struct dentry *lower_new_dentry; > struct dentry *lower_old_dir_dentry; > struct dentry *lower_new_dir_dentry; > + struct dentry *trap = NULL; > > lower_old_dentry = ecryptfs_dentry_to_lower(old_dentry); > lower_new_dentry = ecryptfs_dentry_to_lower(new_dentry); > @@ -621,7 +622,17 @@ ecryptfs_rename(struct inode *old_dir, struct dentry *old_dentry, > dget(lower_new_dentry); > lower_old_dir_dentry = dget_parent(lower_old_dentry); > lower_new_dir_dentry = dget_parent(lower_new_dentry); > - lock_rename(lower_old_dir_dentry, lower_new_dir_dentry); > + trap = lock_rename(lower_old_dir_dentry, lower_new_dir_dentry); > + /* source should not be ancestor of target */ > + if (trap == lower_old_dentry) { > + rc = -EINVAL; > + goto out_lock; > + } > + /* target should not be ancestor of source */ > + if (trap == lower_new_dentry) { > + rc = -ENOTEMPTY; > + goto out_lock; > + } > rc = vfs_rename(lower_old_dir_dentry->d_inode, lower_old_dentry, > lower_new_dir_dentry->d_inode, lower_new_dentry); > if (rc)