Messages in this thread | | | Date | Sat, 11 Oct 2014 17:58:08 -0500 | From | Eric Biggers <> | Subject | fs/namei.c: Misuse of sequence counts? |
| |
Hi, I've been reading through the path lookup code and I believe there may have been bugs introduced by commit 4023bfc9 ("be careful with nd->inode in path_init() and follow_dotdot_rcu()"). And I may have found a pre-existing bug as well.
In follow_dotdot_rcu(), said commit moved loads of the inode to just before read_seqcount_begin(), in several instances. I don't think this is correct, because (as I understand it) read_seqcount_begin() is opening a seq-read critical section on the new dentry. So the inode load should come *after* it, as in the original, to ensure the inode pointer is correctly matched with the sequence count.
In path_init(), said commit added a call to read_seqcount_retry() after loading the inode. I see two problems with this:
- The read_seqcount_retry() isn't needed just to load the inode pointer, so the change doesn't seem to accomplish anything. - If the -ECHILD code path actually runs, the reference to the 'struct file' can be leaked.
Also: if there were actual problems that were "fixed" by this commit, I wonder if they were/are actually caused by the fd-relative case in path_init() using:
nd->seq = __read_seqcount_begin(&nd->path.dentry->d_seq);
instead of
nd->seq = read_seqcount_begin(&nd->path.dentry->d_seq);
since the former is missing a memory barrier before the starting inode is loaded.
Eric
| |