From mboxrd@z Thu Jan 1 00:00:00 1970 From: Artem Bityutskiy Subject: Re: ubifs, race between link and unlink/rename? Date: Fri, 15 May 2009 12:35:59 +0300 Message-ID: <1242380159.27996.230.camel@localhost.localdomain> References: <9917.1242232308@jrobl> Reply-To: dedekind@infradead.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: adrian.hunter@nokia.com, linux-fsdevel@vger.kernel.org To: hooanon05@yahoo.co.jp Return-path: Received: from smtp.nokia.com ([192.100.105.134]:40201 "EHLO mgw-mx09.nokia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1762016AbZEOJgJ (ORCPT ); Fri, 15 May 2009 05:36:09 -0400 In-Reply-To: <9917.1242232308@jrobl> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu, 2009-05-14 at 01:31 +0900, hooanon05@yahoo.co.jp wrote: > Hello, >=20 > Is there a race condition in ubifs? > Here is a scenario. >=20 > Process A Process B > ----------------------+--------------------------- > create("dirA/fileA"); | > unlink("dirA/fileA"); | link("dirA/fileA", "dirB/fileB"); > | unlink("dirB/fileB"); > ----------------------+--------------------------- =46rom: Hunter Adrian Date: Thu, 14 May 2009 06:32:30 +0200 Subject: [PATCH] UBIFS: return error if link and unlink race Consider a scenario when 'vfs_link(dirA/fileA)' and 'vfs_unlink(dirA/fileA, dirB/fileB)' race. 'vfs_link()' does not lock 'dirA->i_mutex', so this is possible. Both of the functions lock 'fileA->i_mutex' though. Suppose 'vfs_unlink()' wins, and takes 'fileA->i_mutex' mutex first. Suppose 'fileA->i_nlink' is 1. In this case 'ubifs_unlink()' will drop the last reference, and put 'inodeA' to the list of orphans. After this, 'vfs_link()' will link 'dirB/fileB' to 'inodeA'. Thir is a problem because, for example, the subsequent 'vfs_unlink(dirB/fileB)' will add the same inode to the list of orphans. This problem was reported by J. R. Okajima [Artem: add more comments, amended commit message] Signed-off-by: Adrian Hunter Signed-off-by: Artem Bityutskiy --- fs/ubifs/dir.c | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-) diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index f55d523..552fb01 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -528,6 +528,25 @@ static int ubifs_link(struct dentry *old_dentry, s= truct inode *dir, inode->i_nlink, dir->i_ino); ubifs_assert(mutex_is_locked(&dir->i_mutex)); ubifs_assert(mutex_is_locked(&inode->i_mutex)); + + /* + * Return -ENOENT if we've raced with unlink and i_nlink is 0. Doing + * otherwise has the potential to corrupt the orphan inode list. + * + * Indeed, consider a scenario when 'vfs_link(dirA/fileA)' and + * 'vfs_unlink(dirA/fileA, dirB/fileB)' race. 'vfs_link()' does not + * lock 'dirA->i_mutex', so this is possible. Both of the functions + * lock 'fileA->i_mutex' though. Suppose 'vfs_unlink()' wins, and tak= es + * 'fileA->i_mutex' mutex first. Suppose 'fileA->i_nlink' is 1. In th= is + * case 'ubifs_unlink()' will drop the last reference, and put 'inode= A' + * to the list of orphans. After this, 'vfs_link()' will link + * 'dirB/fileB' to 'inodeA'. This is a problem because, for example, + * the subsequent 'vfs_unlink(dirB/fileB)' will add the same inode + * to the list of orphans. + */ + if (inode->i_nlink =3D=3D 0) + return -ENOENT; + err =3D dbg_check_synced_i_size(inode); if (err) return err; --=20 1.6.0.6 --=20 Best regards, Artem Bityutskiy (=D0=91=D0=B8=D1=82=D1=8E=D1=86=D0=BA=D0=B8=D0=B9 =D0=90= =D1=80=D1=82=D1=91=D0=BC) -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html