From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zach Brown Subject: Re: [PATCH] vfs: fix vfs_rename_dir for FS_RENAME_DOES_D_MOVE filesystems Date: Fri, 11 Jul 2008 15:12:54 -0700 Message-ID: <4877DAE6.9030405@oracle.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: sage@newdream.net, linux-fsdevel@vger.kernel.org, viro@ZenIV.linux.org.uk, akpm@linux-foundation.org, hch@infradead.org To: Miklos Szeredi Return-path: Received: from tetsuo.zabbo.net ([207.173.201.20]:48027 "EHLO tetsuo.zabbo.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754737AbYGKWMz (ORCPT ); Fri, 11 Jul 2008 18:12:55 -0400 In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Miklos Szeredi wrote: > On Fri, 11 Jul 2008, Sage Weil wrote: >> However, vfs_rename_dir() doesn't properly account for filesystems with >> FS_RENAME_DOES_D_MOVE. If new_dentry has a target inode attached, it >> unhashes the new_dentry prior to the rename() iop and rehashes it after, >> but doesn't account for the possibility that rename() may have swapped >> {old,new}_dentry. For FS_RENAME_DOES_D_MOVE filesystems, it rehashes >> new_dentry (now the old renamed-from name, which d_move() expected to go >> away), such that a subsequent lookup will find it. >> >> To correct this, move vfs_rename_dir()'s call to d_move() _before_ the >> target inode mutex is dealt with. Since d_move() will have been called >> for all filesystems at this point, there is no need to rehash new_dentry >> unless the rename failed. (If the rename succeeded, old_dentry should >> already be rehashed in the new location.) > > I think rehashing the new dentry is bogus, even on error. So we'd just come back through lookup to repopulate the existing destination name that vfs_rename_dir() unhashed before calling ->rename() in the case that the rename fails? That seems gross, but relatively harmless. > So a better fix would be just to remove the rehashing completely. > Does the below patch work for you? It'd work for my case, yeah. - z