On 05/04/2011 11:30 AM, Christoph Hellwig wrote: > On Tue, May 03, 2011 at 04:11:31PM +0200, Gustav Munkby wrote: >> On 05/02/2011 02:40 PM, Al Viro wrote: >>> What happens to rename() with that thing? Specifically, what happens to >>> loop detection? >> >> Good point. >> >> Since my main interest was retrieving contents of a backup, I did not much >> consider the impact of write operations. Before the patch, the source files were >> exposed as normal files, and the target directories as normal directories, so >> one could easily have introduced loops with rename operations before. > > Given that you have the filesystem around can you clarify where the additional > directory links live? I was under the impression they are in hidden backup > directories not normally accessible, so there's always just one link in the > normal namespace. > > The basic idea would be that we allow any kind of renames in the normal > namespace, but disallow any kind of operation involving the time machine > backups. > You are correct, the hardlink targets are all inside a hidden private directory. And then there are hardlink sources both inside and outside this hidden directory linking to directories inside this directory. While the hidden directory is not directly accessible to clients, its subdirectories are through the hardlinks. Perhaps it was obvious to everyone else, but I just realized that it is not enough to prevent moving the directory hardlinks themselves, but all directory renames must be limited (or checked), since the old directory might contain a nested directory hardlink.Disallowing renames of directories altogether is clearly not a viable option. The following three (somewhat overlapping) classes of renames should always be possible without any risk for introducing loops. 1) Allow renames where target directory is an ancestor of source directory. 2) Allow renames that doesn't change the closest hardlink ancestor. 3) Allow renames into target directories without hardlinked parents. The first check seems simple enough to implement, but the other two might require additional locking as far as I understand. Alternatively, they could both be implemented by adding some additional flags/data to dentry->d_fsdata. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
