public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: David Laight <David.Laight@ACULAB.COM>
Cc: 'Jan Kara' <jack@suse.cz>, Ted Tso <tytso@mit.edu>,
	"linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>,
	"Darrick J. Wong" <djwong@kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: [PATCH] ext4: Fix possible corruption when moving a directory with RENAME_EXCHANGE
Date: Wed, 24 May 2023 12:51:48 +0200	[thread overview]
Message-ID: <20230524105148.wgjj7ayrbeol6cdx@quack3> (raw)
In-Reply-To: <48d1f20b2fc1418080c96a1736f6249b@AcuMS.aculab.com>

On Tue 23-05-23 13:50:01, David Laight wrote:
> From: Jan Kara
> > Sent: 23 May 2023 14:14
> > 
> > Commit 0813299c586b ("ext4: Fix possible corruption when moving a
> > directory") forgot that handling of RENAME_EXCHANGE renames needs the
> > protection of inode lock when changing directory parents for moved
> > directories. Add proper locking for that case as well.
> > 
> > CC: stable@vger.kernel.org
> > Fixes: 0813299c586b ("ext4: Fix possible corruption when moving a directory")
> > Reported-by: "Darrick J. Wong" <djwong@kernel.org>
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  fs/ext4/namei.c | 23 +++++++++++++++++++++--
> >  1 file changed, 21 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> > index 45b579805c95..b91abea1c781 100644
> > --- a/fs/ext4/namei.c
> > +++ b/fs/ext4/namei.c
> > @@ -4083,10 +4083,25 @@ static int ext4_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
> >  	if (retval)
> >  		return retval;
> > 
> > +	/*
> > +	 * We need to protect against old.inode and new.inode directory getting
> > +	 * converted from inline directory format into a normal one. The lock
> > +	 * ordering does not matter here as old and new are guaranteed to be
> > +	 * incomparable in the directory hierarchy.
> > +	 */
> > +	if (S_ISDIR(old.inode->i_mode))
> > +		inode_lock(old.inode);
> > +	if (S_ISDIR(new.inode->i_mode))
> > +		inode_lock_nested(new.inode, I_MUTEX_NONDIR2);
> > +
> 
> What happens if there is another concurrent rename from new.inode
> to old.inode?
> That will try to acquire the locks in the other order.

That is not really possible because these two renames cannot happen in
parallel due to VFS locking - either old & new share parent which is locked
by VFS (so there cannot be another rename in that directory) or they have
different parents which are also locked by VFS (so again it is not possible
to race with another rename in these two dirs).

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

  reply	other threads:[~2023-05-24 10:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-23 13:14 [PATCH] ext4: Fix possible corruption when moving a directory with RENAME_EXCHANGE Jan Kara
2023-05-23 13:50 ` David Laight
2023-05-24 10:51   ` Jan Kara [this message]
2023-05-24 13:11     ` Amir Goldstein
2023-05-24 14:18       ` Jan Kara
2023-05-24 14:25         ` David Laight

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=20230524105148.wgjj7ayrbeol6cdx@quack3 \
    --to=jack@suse.cz \
    --cc=David.Laight@ACULAB.COM \
    --cc=djwong@kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=tytso@mit.edu \
    /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