[PATCH 0/4] cifs: Handle all name surrogate reparse points

[PATCH 0/4] cifs: Handle all name surrogate reparse points

[Pali Rohár]

Name surrogate reparse point represents another named entity in the system.

If the name surrogate reparse point is not handled by Linux SMB client
and it is of directory type then treat it as a new mount point.

Cleanup code for all explicit surrogate reparse points (like reparse
points with tag IO_REPARSE_TAG_MOUNT_POINT) as they are handled by
generic name surrogate reparse point code.

Pali Rohár (4):
  cifs: Throw -EOPNOTSUPP error on unsupported reparse point type from
    parse_reparse_point()
  cifs: Treat unhandled directory name surrogate reparse points as mount
    directory nodes
  cifs: Remove explicit handling of IO_REPARSE_TAG_MOUNT_POINT in
    inode.c
  cifs: Improve handling of name surrogate reparse points in reparse.c

 fs/smb/client/inode.c    | 17 +++++++++++++----
 fs/smb/client/reparse.c  | 24 ++++++++++--------------
 fs/smb/common/smbfsctl.h |  3 +++
 3 files changed, 26 insertions(+), 18 deletions(-)

-- 
2.20.1

[PATCH 1/4] cifs: Throw -EOPNOTSUPP error on unsupported reparse point type from parse_reparse_point()

[Pali Rohár]

This would help to track and detect by caller if the reparse point type was
processed or not.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/reparse.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index 4ea987f84bd1..63a95ecc7542 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -1088,13 +1088,12 @@ int parse_reparse_point(struct reparse_data_buffer *buf,
 				 le32_to_cpu(buf->ReparseTag));
 			return -EIO;
 		}
-		break;
+		return 0;
 	default:
 		cifs_tcon_dbg(VFS | ONCE, "unhandled reparse tag: 0x%08x\n",
 			      le32_to_cpu(buf->ReparseTag));
-		break;
+		return -EOPNOTSUPP;
 	}
-	return 0;
 }
 
 int smb2_parse_reparse_point(struct cifs_sb_info *cifs_sb,
-- 
2.20.1

[PATCH 2/4] cifs: Treat unhandled directory name surrogate reparse points as mount directory nodes

[Pali Rohár]

If the reparse point was not handled (indicated by the -EOPNOTSUPP from
ops->parse_reparse_point() call) but reparse tag is of type name surrogate
directory type, then treat is as a new mount point.

Name surrogate reparse point represents another named entity in the system.

From SMB client point of view, this another entity is resolved on the SMB
server, and server serves its content automatically. Therefore from Linux
client point of view, this name surrogate reparse point of directory type
crosses mount point.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/inode.c    | 13 +++++++++++++
 fs/smb/common/smbfsctl.h |  3 +++
 2 files changed, 16 insertions(+)

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 19b6d68007fb..b26403e6fc2c 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1215,6 +1215,19 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data,
 			rc = server->ops->parse_reparse_point(cifs_sb,
 							      full_path,
 							      iov, data);
+			/*
+			 * If the reparse point was not handled but it is the
+			 * name surrogate which points to directory, then treat
+			 * is as a new mount point. Name surrogate reparse point
+			 * represents another named entity in the system.
+			 */
+			if (rc == -EOPNOTSUPP &&
+			    IS_REPARSE_TAG_NAME_SURROGATE(data->reparse.tag) &&
+			    (le32_to_cpu(data->fi.Attributes) & ATTR_DIRECTORY)) {
+				rc = 0;
+				cifs_create_junction_fattr(fattr, sb);
+				goto out;
+			}
 		}
 
 		if (data->reparse.tag == IO_REPARSE_TAG_SYMLINK && !rc) {
diff --git a/fs/smb/common/smbfsctl.h b/fs/smb/common/smbfsctl.h
index 4b379e84c46b..3253a18ecb5c 100644
--- a/fs/smb/common/smbfsctl.h
+++ b/fs/smb/common/smbfsctl.h
@@ -159,6 +159,9 @@
 #define IO_REPARSE_TAG_LX_CHR	     0x80000025
 #define IO_REPARSE_TAG_LX_BLK	     0x80000026
 
+/* If Name Surrogate Bit is set, the file or directory represents another named entity in the system. */
+#define IS_REPARSE_TAG_NAME_SURROGATE(tag) (!!((tag) & 0x20000000))
+
 /* fsctl flags */
 /* If Flags is set to this value, the request is an FSCTL not ioctl request */
 #define SMB2_0_IOCTL_IS_FSCTL		0x00000001
-- 
2.20.1

[PATCH 3/4] cifs: Remove explicit handling of IO_REPARSE_TAG_MOUNT_POINT in inode.c

[Pali Rohár]

IO_REPARSE_TAG_MOUNT_POINT is just a specific case of directory Name
Surrogate reparse point. As reparse_info_to_fattr() already handles all
directory Name Surrogate reparse point (done by the previous change),
there is no need to have explicit case for IO_REPARSE_TAG_MOUNT_POINT.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/inode.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index b26403e6fc2c..ed32d78971f8 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1203,10 +1203,6 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data,
 			goto out;
 		}
 		break;
-	case IO_REPARSE_TAG_MOUNT_POINT:
-		cifs_create_junction_fattr(fattr, sb);
-		rc = 0;
-		goto out;
 	default:
 		/* Check for cached reparse point data */
 		if (data->symlink_target || data->reparse.buf) {
-- 
2.20.1

[PATCH 4/4] cifs: Improve handling of name surrogate reparse points in reparse.c

[Pali Rohár]

Like previous changes for file inode.c, handle directory name surrogate
reparse points generally also in reparse.c.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/reparse.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index 63a95ecc7542..6ffda4455f9b 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -1222,16 +1222,6 @@ bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
 	bool ok;
 
 	switch (tag) {
-	case IO_REPARSE_TAG_INTERNAL:
-		if (!(fattr->cf_cifsattrs & ATTR_DIRECTORY))
-			return false;
-		fallthrough;
-	case IO_REPARSE_TAG_DFS:
-	case IO_REPARSE_TAG_DFSR:
-	case IO_REPARSE_TAG_MOUNT_POINT:
-		/* See cifs_create_junction_fattr() */
-		fattr->cf_mode = S_IFDIR | 0711;
-		break;
 	case IO_REPARSE_TAG_LX_SYMLINK:
 	case IO_REPARSE_TAG_LX_FIFO:
 	case IO_REPARSE_TAG_AF_UNIX:
@@ -1249,7 +1239,14 @@ bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
 		fattr->cf_mode |= S_IFLNK;
 		break;
 	default:
-		return false;
+		if (!(fattr->cf_cifsattrs & ATTR_DIRECTORY))
+			return false;
+		if (!IS_REPARSE_TAG_NAME_SURROGATE(tag) &&
+		    tag != IO_REPARSE_TAG_INTERNAL)
+			return false;
+		/* See cifs_create_junction_fattr() */
+		fattr->cf_mode = S_IFDIR | 0711;
+		break;
 	}
 
 	fattr->cf_dtype = S_DT(fattr->cf_mode);
-- 
2.20.1

Re: [PATCH 0/4] cifs: Handle all name surrogate reparse points

[Pali Rohár]

Hello Steve, I see that you have merged first two changes (1/4 and 2/4)
from this patch series, but the remaining (3/4 and 4/4). Is there any
reason why 3/4 and 4/4 was not taken?

On Sunday 22 December 2024 15:58:41 Pali Rohár wrote:
> Name surrogate reparse point represents another named entity in the system.
> 
> If the name surrogate reparse point is not handled by Linux SMB client
> and it is of directory type then treat it as a new mount point.
> 
> Cleanup code for all explicit surrogate reparse points (like reparse
> points with tag IO_REPARSE_TAG_MOUNT_POINT) as they are handled by
> generic name surrogate reparse point code.
> 
> Pali Rohár (4):
>   cifs: Throw -EOPNOTSUPP error on unsupported reparse point type from
>     parse_reparse_point()
>   cifs: Treat unhandled directory name surrogate reparse points as mount
>     directory nodes
>   cifs: Remove explicit handling of IO_REPARSE_TAG_MOUNT_POINT in
>     inode.c
>   cifs: Improve handling of name surrogate reparse points in reparse.c
> 
>  fs/smb/client/inode.c    | 17 +++++++++++++----
>  fs/smb/client/reparse.c  | 24 ++++++++++--------------
>  fs/smb/common/smbfsctl.h |  3 +++
>  3 files changed, 26 insertions(+), 18 deletions(-)
> 
> -- 
> 2.20.1
> 

Re: [PATCH 0/4] cifs: Handle all name surrogate reparse points

[Steve French]

On Sun, Feb 23, 2025 at 4:23 PM Pali Rohár <pali@kernel.org> wrote:
>
> Hello Steve, I see that you have merged first two changes (1/4 and 2/4)
> from this patch series, but the remaining (3/4 and 4/4). Is there any
> reason why 3/4 and 4/4 was not taken?

Mainly because I wasn't able to easily test it, and didn't get test
feedback for anyone else
on those two who had tried it.

I am ok with looking at them again - and thx for rebasing.   There are
some of the 41
patches in your updated cifs branch that do look suitable or rc5


> On Sunday 22 December 2024 15:58:41 Pali Rohár wrote:
> > Name surrogate reparse point represents another named entity in the system.
> >
> > If the name surrogate reparse point is not handled by Linux SMB client
> > and it is of directory type then treat it as a new mount point.
> >
> > Cleanup code for all explicit surrogate reparse points (like reparse
> > points with tag IO_REPARSE_TAG_MOUNT_POINT) as they are handled by
> > generic name surrogate reparse point code.
> >
> > Pali Rohár (4):
> >   cifs: Throw -EOPNOTSUPP error on unsupported reparse point type from
> >     parse_reparse_point()
> >   cifs: Treat unhandled directory name surrogate reparse points as mount
> >     directory nodes
> >   cifs: Remove explicit handling of IO_REPARSE_TAG_MOUNT_POINT in
> >     inode.c
> >   cifs: Improve handling of name surrogate reparse points in reparse.c
> >
> >  fs/smb/client/inode.c    | 17 +++++++++++++----
> >  fs/smb/client/reparse.c  | 24 ++++++++++--------------
> >  fs/smb/common/smbfsctl.h |  3 +++
> >  3 files changed, 26 insertions(+), 18 deletions(-)
> >
> > --
> > 2.20.1
> >
>


-- 
Thanks,

Steve

Re: [PATCH 0/4] cifs: Handle all name surrogate reparse points

[Pali Rohár]

On Sunday 23 February 2025 18:48:50 Steve French wrote:
> On Sun, Feb 23, 2025 at 4:23 PM Pali Rohár <pali@kernel.org> wrote:
> >
> > Hello Steve, I see that you have merged first two changes (1/4 and 2/4)
> > from this patch series, but the remaining (3/4 and 4/4). Is there any
> > reason why 3/4 and 4/4 was not taken?
> 
> Mainly because I wasn't able to easily test it, and didn't get test
> feedback for anyone else
> on those two who had tried it.
> 
> I am ok with looking at them again - and thx for rebasing.

Ok, when you have a time, please look at them.

> There are some of the 41 patches in your updated cifs branch that do look suitable or rc5

There is "cifs: Change translation of STATUS_DELETE_PENDING to -EBUSY"
which stops returning -ENOENT for directory entry which still exists.

> 
> > On Sunday 22 December 2024 15:58:41 Pali Rohár wrote:
> > > Name surrogate reparse point represents another named entity in the system.
> > >
> > > If the name surrogate reparse point is not handled by Linux SMB client
> > > and it is of directory type then treat it as a new mount point.
> > >
> > > Cleanup code for all explicit surrogate reparse points (like reparse
> > > points with tag IO_REPARSE_TAG_MOUNT_POINT) as they are handled by
> > > generic name surrogate reparse point code.
> > >
> > > Pali Rohár (4):
> > >   cifs: Throw -EOPNOTSUPP error on unsupported reparse point type from
> > >     parse_reparse_point()
> > >   cifs: Treat unhandled directory name surrogate reparse points as mount
> > >     directory nodes
> > >   cifs: Remove explicit handling of IO_REPARSE_TAG_MOUNT_POINT in
> > >     inode.c
> > >   cifs: Improve handling of name surrogate reparse points in reparse.c
> > >
> > >  fs/smb/client/inode.c    | 17 +++++++++++++----
> > >  fs/smb/client/reparse.c  | 24 ++++++++++--------------
> > >  fs/smb/common/smbfsctl.h |  3 +++
> > >  3 files changed, 26 insertions(+), 18 deletions(-)
> > >
> > > --
> > > 2.20.1
> > >
> >
> 
> 
> -- 
> Thanks,
> 
> Steve

Re: [PATCH 0/4] cifs: Handle all name surrogate reparse points

[Steve French]

On Sun, Mar 2, 2025 at 6:25 AM Pali Rohár <pali@kernel.org> wrote:
>
> On Sunday 23 February 2025 18:48:50 Steve French wrote:
> > On Sun, Feb 23, 2025 at 4:23 PM Pali Rohár <pali@kernel.org> wrote:
> > >
> > > Hello Steve, I see that you have merged first two changes (1/4 and 2/4)
> > > from this patch series, but the remaining (3/4 and 4/4). Is there any
> > > reason why 3/4 and 4/4 was not taken?
> >
> > Mainly because I wasn't able to easily test it, and didn't get test
> > feedback for anyone else
> > on those two who had tried it.
> >
> > I am ok with looking at them again - and thx for rebasing.
>
> Ok, when you have a time, please look at them.
>
> > There are some of the 41 patches in your updated cifs branch that do look suitable or rc5
>
> There is "cifs: Change translation of STATUS_DELETE_PENDING to -EBUSY"
> which stops returning -ENOENT for directory entry which still exists.

IIRC - there were some objections to this if it could break any
plausible existing application behavior, but will need to dig into the
thread from earlier.

Tom or Paulo,
Do you remember if this is one that you had mentioned?



-- 
Thanks,

Steve

Re: [PATCH 0/4] cifs: Handle all name surrogate reparse points

[Pali Rohár]

On Sunday 02 March 2025 19:01:00 Steve French wrote:
> On Sun, Mar 2, 2025 at 6:25 AM Pali Rohár <pali@kernel.org> wrote:
> >
> > On Sunday 23 February 2025 18:48:50 Steve French wrote:
> > > On Sun, Feb 23, 2025 at 4:23 PM Pali Rohár <pali@kernel.org> wrote:
> > > >
> > > > Hello Steve, I see that you have merged first two changes (1/4 and 2/4)
> > > > from this patch series, but the remaining (3/4 and 4/4). Is there any
> > > > reason why 3/4 and 4/4 was not taken?
> > >
> > > Mainly because I wasn't able to easily test it, and didn't get test
> > > feedback for anyone else
> > > on those two who had tried it.
> > >
> > > I am ok with looking at them again - and thx for rebasing.
> >
> > Ok, when you have a time, please look at them.
> >
> > > There are some of the 41 patches in your updated cifs branch that do look suitable or rc5
> >
> > There is "cifs: Change translation of STATUS_DELETE_PENDING to -EBUSY"
> > which stops returning -ENOENT for directory entry which still exists.
> 
> IIRC - there were some objections to this if it could break any
> plausible existing application behavior, but will need to dig into the
> thread from earlier.
> 
> Tom or Paulo,
> Do you remember if this is one that you had mentioned?

I have not figured out any regression for the
STATUS_DELETE_PENDING/EBUSY change.

If you have some scenario or other test case for it then please let me
know what can be wrong here.

I think that it should not cause any regression because applications on
ENOENT error cannot expect that the dir entry still existing.