| Message ID | 20170222000629.7995-1-tuomas@tuxera.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Wed, 22 Feb 2017 02:06:29 +0200 Tuomas Tynkkynen <tuomas@tuxera.com> wrote: > Commit fd2421f54423 ("fs/9p: When doing inode lookup compare qid details > and inode mode bits.") transformed v9fs_qid_iget() to use iget5_locked() > instead of iget_locked(). However, the test() callback is not checking > fid.path at all, which means that a lookup in the inode cache can now > accidentally locate a completely wrong inode from the same inode hash > bucket if the other fields (qid.type and qid.version) match. > Al, does this sound sensible? (or if there is someone else picking up 9p patches, let me know). Thanks! - Tuomas ------------------------------------------------------------------------------ Announcing the Oxford Dictionaries API! The API offers world-renowned dictionary content that is easy and intuitive to access. Sign up for an account today to start using our lexical data to power your apps and projects. Get started today and enter our developer competition. http://sdm.link/oxford
Reviewed-by: Latchesar Ionkov <lucho@ionkov.net> On Tue, Feb 21, 2017 at 6:06 PM, Tuomas Tynkkynen <tuomas@tuxera.com> wrote: > Commit fd2421f54423 ("fs/9p: When doing inode lookup compare qid details > and inode mode bits.") transformed v9fs_qid_iget() to use iget5_locked() > instead of iget_locked(). However, the test() callback is not checking > fid.path at all, which means that a lookup in the inode cache can now > accidentally locate a completely wrong inode from the same inode hash > bucket if the other fields (qid.type and qid.version) match. > > Fixes: fd2421f54423 ("fs/9p: When doing inode lookup compare qid details and inode mode bits.") > Cc: stable@vger.kernel.org > Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com> > --- > Does this sound sensible? I have never reproduced the problem myself but > reportedly this patch solves some really weird problems where the > symptoms match (wrong files being opened unpredictably). > --- > fs/9p/vfs_inode.c | 3 +++ > fs/9p/vfs_inode_dotl.c | 3 +++ > 2 files changed, 6 insertions(+) > > diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c > index 30ca770c5e0b..f8ab4a66acaf 100644 > --- a/fs/9p/vfs_inode.c > +++ b/fs/9p/vfs_inode.c > @@ -483,6 +483,9 @@ static int v9fs_test_inode(struct inode *inode, void *data) > > if (v9inode->qid.type != st->qid.type) > return 0; > + > + if (v9inode->qid.path != st->qid.path) > + return 0; > return 1; > } > > diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c > index afaa4b6de801..c3dd0d42bb3a 100644 > --- a/fs/9p/vfs_inode_dotl.c > +++ b/fs/9p/vfs_inode_dotl.c > @@ -87,6 +87,9 @@ static int v9fs_test_inode_dotl(struct inode *inode, void *data) > > if (v9inode->qid.type != st->qid.type) > return 0; > + > + if (v9inode->qid.path != st->qid.path) > + return 0; > return 1; > } > > -- > 2.11.1 > ------------------------------------------------------------------------------ Announcing the Oxford Dictionaries API! The API offers world-renowned dictionary content that is easy and intuitive to access. Sign up for an account today to start using our lexical data to power your apps and projects. Get started today and enter our developer competition. http://sdm.link/oxford
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c index 30ca770c5e0b..f8ab4a66acaf 100644 --- a/fs/9p/vfs_inode.c +++ b/fs/9p/vfs_inode.c @@ -483,6 +483,9 @@ static int v9fs_test_inode(struct inode *inode, void *data) if (v9inode->qid.type != st->qid.type) return 0; + + if (v9inode->qid.path != st->qid.path) + return 0; return 1; } diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c index afaa4b6de801..c3dd0d42bb3a 100644 --- a/fs/9p/vfs_inode_dotl.c +++ b/fs/9p/vfs_inode_dotl.c @@ -87,6 +87,9 @@ static int v9fs_test_inode_dotl(struct inode *inode, void *data) if (v9inode->qid.type != st->qid.type) return 0; + + if (v9inode->qid.path != st->qid.path) + return 0; return 1; }
Commit fd2421f54423 ("fs/9p: When doing inode lookup compare qid details and inode mode bits.") transformed v9fs_qid_iget() to use iget5_locked() instead of iget_locked(). However, the test() callback is not checking fid.path at all, which means that a lookup in the inode cache can now accidentally locate a completely wrong inode from the same inode hash bucket if the other fields (qid.type and qid.version) match. Fixes: fd2421f54423 ("fs/9p: When doing inode lookup compare qid details and inode mode bits.") Cc: stable@vger.kernel.org Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com> --- Does this sound sensible? I have never reproduced the problem myself but reportedly this patch solves some really weird problems where the symptoms match (wrong files being opened unpredictably). --- fs/9p/vfs_inode.c | 3 +++ fs/9p/vfs_inode_dotl.c | 3 +++ 2 files changed, 6 insertions(+)