lkml.org 
[lkml]   [2014]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
Subjectfs/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


\
 
 \ /
  Last update: 2014-10-12 01:21    [W:0.058 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site