| Message ID | 1427982695-29891-1-git-send-email-sprabhu@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2 Apr 2015 14:51:35 +0100 Sachin Prabhu <sprabhu@redhat.com> wrote: > Doing a readdir on a dfs root can result in the dentries for directories > with a dfs share mounted being replaced by new dentries for objects > returned by the readdir call. These new dentries on shares mounted with > unix extenstions show up as symlinks pointing to the dfs share. > > # mount -t cifs -o sec=none //vm140-31/dfsroot cifs > # stat cifs/testlink/testfile; ls -l cifs > File: ‘cifs/testlink/testfile’ > Size: 0 Blocks: 0 IO Block: 16384 regular > empty file > Device: 27h/39d Inode: 130120 Links: 1 > Access: (0644/-rw-r--r--) Uid: ( 0/ root) Gid: ( 0/ root) > Access: 2015-03-31 13:55:50.106018200 +0100 > Modify: 2015-03-31 13:55:50.106018200 +0100 > Change: 2015-03-31 13:55:50.106018200 +0100 > Birth: - > total 0 > drwxr-xr-x 2 root root 0 Mar 31 13:54 testdir > lrwxrwxrwx 1 root root 19 Mar 24 14:25 testlink -> \vm140-31\test > > In the example above, the stat command mounts the dfs share at > cifs/testlink. The subsequent ls on the dfsroot directory replaces the > dentry for testlink with a symlink. > > In the earlier code, the d_invalidate command returned an -EBUSY error > when attempting to invalidate directories. This stopped the code from > replacing the directories with symlinks returned by the readdir call. > Changes were recently made to the d_invalidate() command so > that it no longer returns an error code. This results in the directory > with the mounted dfs share being replaced by a symlink which denotes a > dfs share. > > Signed-off-by: Sachin Prabhu <sprabhu@redhat.com> > --- > fs/cifs/readdir.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c > index c295338..b4bda47 100644 > --- a/fs/cifs/readdir.c > +++ b/fs/cifs/readdir.c > @@ -90,6 +90,8 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name, > if (dentry) { > inode = dentry->d_inode; > if (inode) { > + if (d_mountpoint(dentry)) > + goto out; > /* > * If we're generating inode numbers, then we don't > * want to clobber the existing one with the one that Looks right. Reviewed-by: Jeff Layton <jeff.layton@primarydata.com> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Looks reasonable but fails on SMB3 - we need to fix lines 133 and 134 of smb2pdu.c as well On Fri, Apr 3, 2015 at 7:14 AM, Jeff Layton <jeff.layton@primarydata.com> wrote: > On Thu, 2 Apr 2015 14:51:35 +0100 > Sachin Prabhu <sprabhu@redhat.com> wrote: > >> Doing a readdir on a dfs root can result in the dentries for directories >> with a dfs share mounted being replaced by new dentries for objects >> returned by the readdir call. These new dentries on shares mounted with >> unix extenstions show up as symlinks pointing to the dfs share. >> >> # mount -t cifs -o sec=none //vm140-31/dfsroot cifs >> # stat cifs/testlink/testfile; ls -l cifs >> File: ‘cifs/testlink/testfile’ >> Size: 0 Blocks: 0 IO Block: 16384 regular >> empty file >> Device: 27h/39d Inode: 130120 Links: 1 >> Access: (0644/-rw-r--r--) Uid: ( 0/ root) Gid: ( 0/ root) >> Access: 2015-03-31 13:55:50.106018200 +0100 >> Modify: 2015-03-31 13:55:50.106018200 +0100 >> Change: 2015-03-31 13:55:50.106018200 +0100 >> Birth: - >> total 0 >> drwxr-xr-x 2 root root 0 Mar 31 13:54 testdir >> lrwxrwxrwx 1 root root 19 Mar 24 14:25 testlink -> \vm140-31\test >> >> In the example above, the stat command mounts the dfs share at >> cifs/testlink. The subsequent ls on the dfsroot directory replaces the >> dentry for testlink with a symlink. >> >> In the earlier code, the d_invalidate command returned an -EBUSY error >> when attempting to invalidate directories. This stopped the code from >> replacing the directories with symlinks returned by the readdir call. >> Changes were recently made to the d_invalidate() command so >> that it no longer returns an error code. This results in the directory >> with the mounted dfs share being replaced by a symlink which denotes a >> dfs share. >> >> Signed-off-by: Sachin Prabhu <sprabhu@redhat.com> >> --- >> fs/cifs/readdir.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c >> index c295338..b4bda47 100644 >> --- a/fs/cifs/readdir.c >> +++ b/fs/cifs/readdir.c >> @@ -90,6 +90,8 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name, >> if (dentry) { >> inode = dentry->d_inode; >> if (inode) { >> + if (d_mountpoint(dentry)) >> + goto out; >> /* >> * If we're generating inode numbers, then we don't >> * want to clobber the existing one with the one that > > > Looks right. > > Reviewed-by: Jeff Layton <jeff.layton@primarydata.com> > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2015-04-03 at 20:57 -0500, Steve French wrote: > Looks reasonable but fails on SMB3 - we need to fix lines 133 and 134 > of smb2pdu.c as well Hello Steve, I am not sure I understand how these lines are involved. The patch fixes behaviour for readdir where the lookup data doesn't contain any information indicating that the directory leads to a DFS lookup. Sachin Prabhu > > On Fri, Apr 3, 2015 at 7:14 AM, Jeff Layton <jeff.layton@primarydata.com> wrote: > > On Thu, 2 Apr 2015 14:51:35 +0100 > > Sachin Prabhu <sprabhu@redhat.com> wrote: > > > >> Doing a readdir on a dfs root can result in the dentries for directories > >> with a dfs share mounted being replaced by new dentries for objects > >> returned by the readdir call. These new dentries on shares mounted with > >> unix extenstions show up as symlinks pointing to the dfs share. > >> > >> # mount -t cifs -o sec=none //vm140-31/dfsroot cifs > >> # stat cifs/testlink/testfile; ls -l cifs > >> File: ‘cifs/testlink/testfile’ > >> Size: 0 Blocks: 0 IO Block: 16384 regular > >> empty file > >> Device: 27h/39d Inode: 130120 Links: 1 > >> Access: (0644/-rw-r--r--) Uid: ( 0/ root) Gid: ( 0/ root) > >> Access: 2015-03-31 13:55:50.106018200 +0100 > >> Modify: 2015-03-31 13:55:50.106018200 +0100 > >> Change: 2015-03-31 13:55:50.106018200 +0100 > >> Birth: - > >> total 0 > >> drwxr-xr-x 2 root root 0 Mar 31 13:54 testdir > >> lrwxrwxrwx 1 root root 19 Mar 24 14:25 testlink -> \vm140-31\test > >> > >> In the example above, the stat command mounts the dfs share at > >> cifs/testlink. The subsequent ls on the dfsroot directory replaces the > >> dentry for testlink with a symlink. > >> > >> In the earlier code, the d_invalidate command returned an -EBUSY error > >> when attempting to invalidate directories. This stopped the code from > >> replacing the directories with symlinks returned by the readdir call. > >> Changes were recently made to the d_invalidate() command so > >> that it no longer returns an error code. This results in the directory > >> with the mounted dfs share being replaced by a symlink which denotes a > >> dfs share. > >> > >> Signed-off-by: Sachin Prabhu <sprabhu@redhat.com> > >> --- > >> fs/cifs/readdir.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c > >> index c295338..b4bda47 100644 > >> --- a/fs/cifs/readdir.c > >> +++ b/fs/cifs/readdir.c > >> @@ -90,6 +90,8 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name, > >> if (dentry) { > >> inode = dentry->d_inode; > >> if (inode) { > >> + if (d_mountpoint(dentry)) > >> + goto out; > >> /* > >> * If we're generating inode numbers, then we don't > >> * want to clobber the existing one with the one that > > > > > > Looks right. > > > > Reviewed-by: Jeff Layton <jeff.layton@primarydata.com> > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 7, 2015 at 6:26 AM, Sachin Prabhu <sprabhu@redhat.com> wrote: > On Fri, 2015-04-03 at 20:57 -0500, Steve French wrote: >> Looks reasonable but fails on SMB3 - we need to fix lines 133 and 134 >> of smb2pdu.c as well > > Hello Steve, > > I am not sure I understand how these lines are involved. > The patch fixes behaviour for readdir where the lookup data doesn't > contain any information indicating that the directory leads to a DFS > lookup. I already merged your patch and don't see a problem with those lines - but was just noting that I could only test it on cifs - we can't test it in the more important use cases (SMB2.1, SMB3) due to what I pointed out in the earlier note.
diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c index c295338..b4bda47 100644 --- a/fs/cifs/readdir.c +++ b/fs/cifs/readdir.c @@ -90,6 +90,8 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name, if (dentry) { inode = dentry->d_inode; if (inode) { + if (d_mountpoint(dentry)) + goto out; /* * If we're generating inode numbers, then we don't * want to clobber the existing one with the one that
Doing a readdir on a dfs root can result in the dentries for directories with a dfs share mounted being replaced by new dentries for objects returned by the readdir call. These new dentries on shares mounted with unix extenstions show up as symlinks pointing to the dfs share. # mount -t cifs -o sec=none //vm140-31/dfsroot cifs # stat cifs/testlink/testfile; ls -l cifs File: ‘cifs/testlink/testfile’ Size: 0 Blocks: 0 IO Block: 16384 regular empty file Device: 27h/39d Inode: 130120 Links: 1 Access: (0644/-rw-r--r--) Uid: ( 0/ root) Gid: ( 0/ root) Access: 2015-03-31 13:55:50.106018200 +0100 Modify: 2015-03-31 13:55:50.106018200 +0100 Change: 2015-03-31 13:55:50.106018200 +0100 Birth: - total 0 drwxr-xr-x 2 root root 0 Mar 31 13:54 testdir lrwxrwxrwx 1 root root 19 Mar 24 14:25 testlink -> \vm140-31\test In the example above, the stat command mounts the dfs share at cifs/testlink. The subsequent ls on the dfsroot directory replaces the dentry for testlink with a symlink. In the earlier code, the d_invalidate command returned an -EBUSY error when attempting to invalidate directories. This stopped the code from replacing the directories with symlinks returned by the readdir call. Changes were recently made to the d_invalidate() command so that it no longer returns an error code. This results in the directory with the mounted dfs share being replaced by a symlink which denotes a dfs share. Signed-off-by: Sachin Prabhu <sprabhu@redhat.com> --- fs/cifs/readdir.c | 2 ++ 1 file changed, 2 insertions(+)