From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrian Hunter Subject: Re: ubifs, race between link and unlink/rename? Date: Wed, 13 May 2009 21:28:37 +0300 Message-ID: <4A0B1155.4060906@nokia.com> References: <9917.1242232308@jrobl> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: "dedekind@infradead.org" , "linux-fsdevel@vger.kernel.org" , hch@lst.de To: "hooanon05@yahoo.co.jp" Return-path: Received: from smtp.nokia.com ([192.100.122.230]:24768 "EHLO mgw-mx03.nokia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756000AbZEMS14 (ORCPT ); Wed, 13 May 2009 14:27:56 -0400 In-Reply-To: <9917.1242232308@jrobl> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: hooanon05@yahoo.co.jp wrote: > Is there a race condition in ubifs? Yes, it does look like it. EXT3 and EXT4 explicitly check for nlink == 0 in their link functions. UBIFS seems to have overlooked that check. I guess some file systems do not have a problem with the race, which is why VFS allows it. We will patch UBIFS, but I cannot comment about changing VFS. Thank you for finding this :-) > Here is a scenario. > > Process A Process B > ----------------------+--------------------------- > create("dirA/fileA"); | > unlink("dirA/fileA"); | link("dirA/fileA", "dirB/fileB"); > | unlink("dirB/fileB"); > ----------------------+--------------------------- > > In link(2), dirA->i_mutex is not held. So unlink("dirA/fileA") can run > concurrently (after lookup). > While ubifs acquires ubifs_inode->ui_mutex, it doesn't check i_nlink. > When unlink("dirA/fileA") wins ui_mutex race and link() loses, > - ubifs_unlink("dirA/fileA") will call ubifs_jnl_update/ubifs_add_orphan() > - ubifs_link() will operate the inode with i_nlink == 0 (finally it will > be 1) > - ubifs_unlink("dirB/fileB") will call ubifs_add_orphan() again for the > same inode > - and it will produce "orphaned twice" error. > - (ubifs_add_orphan() is called by ubifs_rename/ubifs_jnl_rename() too) > > If this scenario is possible, it may happen in every FS. > To check i_nlink in vfs_link (like this) may be one option, but it might > be better to check in ubifs since ubifs_add_orphan() is for i_nlink == 0 > and ubifs specific. > > diff --git a/fs/namei.c b/fs/namei.c > index b207821..820c386 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -2409,9 +2409,12 @@ int vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_de > if (error) > return error; > > + error = -ENOENT; > mutex_lock(&inode->i_mutex); > - DQUOT_INIT(dir); > - error = dir->i_op->link(old_dentry, dir, new_dentry); > + if (inode->i_nlink) { > + DQUOT_INIT(dir); > + error = dir->i_op->link(old_dentry, dir, new_dentry); > + } > mutex_unlock(&inode->i_mutex); > if (!error) > fsnotify_link(dir, inode, new_dentry); > > > J. R. Okajima >