[PATCH 0/8] cifs: Fix support for NFS-style reparse points

[PATCH 0/8] cifs: Fix support for NFS-style reparse points

[Pali Rohár]

For NFS-style reparse points in the current Linux SMB client I found few
buffer overflows and then incompatibility issues related to char/block
devices and symlinks. In this patch series I'm addressing these issues.
I also located commits which introduced these issues, I put them into
Fixes lines of commit messages.

Test cases against Windows server which exports one directory over both
SMB and NFS protocols. On Linux is mounted that directory to /mnt/nfs
and /mnt/smb via different protocols.

  mknod /mnt/nfs/char c 1 3
  stat /mnt/smb/char
  mknod /mnt/nfs/block b 8 0
  stat /mnt/smb/block
  ln -s abc\\abc /mnt/nfs/symlink
  stat /mnt/smb/symlink
  ls -l /mnt/smb

ls -l or stat over SMB should show the same information about char, block
and symlink as over NFS. And vice-versa.

Please look and check the buffer overflow issue as these buffer lengths
are always nightmares to handle correctly.

Pali Rohár (8):
  smb: Update comments about some reparse point tags
  cifs: Remove intermediate object of failed create reparse call
  cifs: Fix parsing NFS-style char/block devices
  cifs: Fix creating NFS-style char/block devices
  cifs: Fix buffer overflow when parsing NFS reparse points
  cifs: Do not convert delimiter when parsing NFS-style symlinks
  cifs: Validate content of NFS reparse point buffer
  cifs: Rename posix to nfs in parse_reparse_posix() and
    reparse_posix_data

 fs/smb/client/cifsglob.h  |  2 +-
 fs/smb/client/cifspdu.h   |  2 +-
 fs/smb/client/reparse.c   | 53 +++++++++++++++++++++++++++++++--------
 fs/smb/client/reparse.h   | 12 ++++++---
 fs/smb/client/smb2inode.c | 21 ++++++++++++++--
 fs/smb/common/smb2pdu.h   |  2 +-
 fs/smb/common/smbfsctl.h  |  7 +++---
 7 files changed, 77 insertions(+), 22 deletions(-)

-- 
2.20.1

[PATCH 1/8] smb: Update comments about some reparse point tags

[Pali Rohár]

NFS-style reparse points are recognized only by the Windows NFS server 2012
and new. Windows 8 does not contain Windows NFS server, so these reparse
points are not used on Windows 8.

Reparse points with IO_REPARSE_TAG_AF_UNIX tag were primarily introduced for
native Win32 AF_UNIX sockets and later were re-used by also by WSL:
https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
https://devblogs.microsoft.com/commandline/windowswsl-interop-with-af_unix/

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/common/smbfsctl.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/smb/common/smbfsctl.h b/fs/smb/common/smbfsctl.h
index a94d658b88e8..4b379e84c46b 100644
--- a/fs/smb/common/smbfsctl.h
+++ b/fs/smb/common/smbfsctl.h
@@ -140,20 +140,21 @@
 /* Used by the DFS filter See MS-DFSC */
 #define IO_REPARSE_TAG_DFSR          0x80000012
 #define IO_REPARSE_TAG_FILTER_MANAGER 0x8000000B
-/* See section MS-FSCC 2.1.2.4 */
+/* Native SMB symlinks since Windows Vista, see MS-FSCC 2.1.2.4 */
 #define IO_REPARSE_TAG_SYMLINK       0xA000000C
 #define IO_REPARSE_TAG_DEDUP         0x80000013
 #define IO_REPARSE_APPXSTREAM	     0xC0000014
-/* NFS symlinks, Win 8/SMB3 and later */
+/* NFS special files used by Windows NFS server since Windows Server 2012, see MS-FSCC 2.1.2.6 */
 #define IO_REPARSE_TAG_NFS           0x80000014
 /*
  * AzureFileSync - see
  * https://docs.microsoft.com/en-us/azure/storage/files/storage-sync-cloud-tiering
  */
 #define IO_REPARSE_TAG_AZ_FILE_SYNC  0x8000001e
+/* Native Win32 AF_UNIX sockets since Windows 10 April 2018 Update, used also by WSL */
+#define IO_REPARSE_TAG_AF_UNIX       0x80000023
 /* WSL reparse tags */
 #define IO_REPARSE_TAG_LX_SYMLINK    0xA000001D
-#define IO_REPARSE_TAG_AF_UNIX	     0x80000023
 #define IO_REPARSE_TAG_LX_FIFO	     0x80000024
 #define IO_REPARSE_TAG_LX_CHR	     0x80000025
 #define IO_REPARSE_TAG_LX_BLK	     0x80000026
-- 
2.20.1

[PATCH 2/8] cifs: Remove intermediate object of failed create reparse call

[Pali Rohár]

If CREATE was successful but SMB2_OP_SET_REPARSE failed then remove the
intermediate object created by CREATE. Otherwise empty object stay on the
server when reparse call failed.

This ensures that if the creating of special files is unsupported by the
server then no empty file stay on the server as a result of unsupported
operation.

Fixes: 102466f303ff ("smb: client: allow creating special files via reparse points")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/smb2inode.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 11a1c53c64e0..af42f44bdcf4 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -1205,6 +1205,8 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
 	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
 	struct cifsFileInfo *cfile;
 	struct inode *new = NULL;
+	int out_buftype[2] = {};
+	struct kvec out_iov[2];
 	struct kvec in_iov[2];
 	int cmds[2];
 	int rc;
@@ -1228,7 +1230,7 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
 		cmds[1] = SMB2_OP_POSIX_QUERY_INFO;
 		cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
 		rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
-				      in_iov, cmds, 2, cfile, NULL, NULL, NULL);
+				      in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
 		if (!rc) {
 			rc = smb311_posix_get_inode_info(&new, full_path,
 							 data, sb, xid);
@@ -1237,12 +1239,27 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
 		cmds[1] = SMB2_OP_QUERY_INFO;
 		cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
 		rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
-				      in_iov, cmds, 2, cfile, NULL, NULL, NULL);
+				      in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
 		if (!rc) {
 			rc = cifs_get_inode_info(&new, full_path,
 						 data, sb, xid, NULL);
 		}
 	}
+
+	if (rc) {
+		/*
+		 * If CREATE was successful but SMB2_OP_SET_REPARSE failed then
+		 * remove the intermediate object created by CREATE. Otherwise
+		 * empty object stay on the server when reparse call failed.
+		 */
+		if (((struct smb2_hdr *)out_iov[0].iov_base)->Status == STATUS_SUCCESS &&
+		    ((struct smb2_hdr *)out_iov[1].iov_base)->Status != STATUS_SUCCESS)
+			smb2_unlink(xid, tcon, full_path, cifs_sb, NULL);
+	}
+
+	free_rsp_buf(out_buftype[0], out_iov[0].iov_base);
+	free_rsp_buf(out_buftype[1], out_iov[1].iov_base);
+
 	return rc ? ERR_PTR(rc) : new;
 }
 
-- 
2.20.1

Re: [PATCH 2/8] cifs: Remove intermediate object of failed create reparse call

[Pali Rohár]

Hello Steve, I was reading again implementation of smb2_compound_op()
function and if I understood it correctly, then out_iov and out_buftype
array parameters needs to have num_cmds+2 members. This is quite
unexpected, and this patch cause buffer overflow and memory leaks.
+2 is for CREATE and CLOSE operations added automatically by compound.

Surprisingly, no memory issue and neither corrupted packed I observed.
And therefore I thought that change is working fine.

I will send a new version of this change to increase array members and
free all members of array.

On Saturday 28 September 2024 23:59:42 Pali Rohár wrote:
> If CREATE was successful but SMB2_OP_SET_REPARSE failed then remove the
> intermediate object created by CREATE. Otherwise empty object stay on the
> server when reparse call failed.
> 
> This ensures that if the creating of special files is unsupported by the
> server then no empty file stay on the server as a result of unsupported
> operation.
> 
> Fixes: 102466f303ff ("smb: client: allow creating special files via reparse points")
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  fs/smb/client/smb2inode.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> index 11a1c53c64e0..af42f44bdcf4 100644
> --- a/fs/smb/client/smb2inode.c
> +++ b/fs/smb/client/smb2inode.c
> @@ -1205,6 +1205,8 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
>  	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
>  	struct cifsFileInfo *cfile;
>  	struct inode *new = NULL;
> +	int out_buftype[2] = {};
> +	struct kvec out_iov[2];
>  	struct kvec in_iov[2];
>  	int cmds[2];
>  	int rc;
> @@ -1228,7 +1230,7 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
>  		cmds[1] = SMB2_OP_POSIX_QUERY_INFO;
>  		cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
>  		rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
> -				      in_iov, cmds, 2, cfile, NULL, NULL, NULL);
> +				      in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
>  		if (!rc) {
>  			rc = smb311_posix_get_inode_info(&new, full_path,
>  							 data, sb, xid);
> @@ -1237,12 +1239,27 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
>  		cmds[1] = SMB2_OP_QUERY_INFO;
>  		cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
>  		rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
> -				      in_iov, cmds, 2, cfile, NULL, NULL, NULL);
> +				      in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
>  		if (!rc) {
>  			rc = cifs_get_inode_info(&new, full_path,
>  						 data, sb, xid, NULL);
>  		}
>  	}
> +
> +	if (rc) {
> +		/*
> +		 * If CREATE was successful but SMB2_OP_SET_REPARSE failed then
> +		 * remove the intermediate object created by CREATE. Otherwise
> +		 * empty object stay on the server when reparse call failed.
> +		 */
> +		if (((struct smb2_hdr *)out_iov[0].iov_base)->Status == STATUS_SUCCESS &&
> +		    ((struct smb2_hdr *)out_iov[1].iov_base)->Status != STATUS_SUCCESS)
> +			smb2_unlink(xid, tcon, full_path, cifs_sb, NULL);
> +	}
> +
> +	free_rsp_buf(out_buftype[0], out_iov[0].iov_base);
> +	free_rsp_buf(out_buftype[1], out_iov[1].iov_base);
> +
>  	return rc ? ERR_PTR(rc) : new;
>  }
>  
> -- 
> 2.20.1
> 

[PATCH v2] cifs: Remove intermediate object of failed create reparse call

[Pali Rohár]

If CREATE was successful but SMB2_OP_SET_REPARSE failed then remove the
intermediate object created by CREATE. Otherwise empty object stay on the
server when reparse call failed.

This ensures that if the creating of special files is unsupported by the
server then no empty file stay on the server as a result of unsupported
operation.

Fixes: 102466f303ff ("smb: client: allow creating special files via reparse points")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
Changes in v2:
* Increase out_buftype[] and out_iov[] members from 2 to 4 as required by smb2_compound_op
* Call free_rsp_buf() for all members of out_buftype[]/out_iov[]

I would like if you double check this smb2_compound_op() usage if there
is not some other memory issue. As V1 contained both memory leak and
buffer overflow (smb2_compound_op wrote out of those two arrays).

---
 fs/smb/client/smb2inode.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 11a1c53c64e0..6e69a3b98be3 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -1205,9 +1205,12 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
 	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
 	struct cifsFileInfo *cfile;
 	struct inode *new = NULL;
+	int out_buftype[4] = {};
+	struct kvec out_iov[4] = {};
 	struct kvec in_iov[2];
 	int cmds[2];
 	int rc;
+	int i;
 
 	oparms = CIFS_OPARMS(cifs_sb, tcon, full_path,
 			     SYNCHRONIZE | DELETE |
@@ -1228,7 +1231,7 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
 		cmds[1] = SMB2_OP_POSIX_QUERY_INFO;
 		cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
 		rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
-				      in_iov, cmds, 2, cfile, NULL, NULL, NULL);
+				      in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
 		if (!rc) {
 			rc = smb311_posix_get_inode_info(&new, full_path,
 							 data, sb, xid);
@@ -1237,12 +1240,27 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
 		cmds[1] = SMB2_OP_QUERY_INFO;
 		cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
 		rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
-				      in_iov, cmds, 2, cfile, NULL, NULL, NULL);
+				      in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
 		if (!rc) {
 			rc = cifs_get_inode_info(&new, full_path,
 						 data, sb, xid, NULL);
 		}
 	}
+
+	if (rc) {
+		/*
+		 * If CREATE was successful but SMB2_OP_SET_REPARSE failed then
+		 * remove the intermediate object created by CREATE. Otherwise
+		 * empty object stay on the server when reparse call failed.
+		 */
+		if (((struct smb2_hdr *)out_iov[0].iov_base)->Status == STATUS_SUCCESS &&
+		    ((struct smb2_hdr *)out_iov[1].iov_base)->Status != STATUS_SUCCESS)
+			smb2_unlink(xid, tcon, full_path, cifs_sb, NULL);
+	}
+
+	for (i = 0; i < ARRAY_SIZE(out_buftype); i++)
+		free_rsp_buf(out_buftype[i], out_iov[i].iov_base);
+
 	return rc ? ERR_PTR(rc) : new;
 }
 
-- 
2.20.1

Re: [PATCH v2] cifs: Remove intermediate object of failed create reparse call

[Steve French]

Running regression tests on these currently.  Let me know if
additional patches to add for these experiments

1c2fcb28ce99 (HEAD -> for-next, origin/for-next, origin/HEAD) cifs: Do
not convert delimiter when parsing NFS-style symlinks
92484193d70a cifs: Validate content of NFS reparse point buffer
c77a8e49f2d3 cifs: Fix buffer overflow when parsing NFS reparse points
ab7d68fd4bcc cifs: Remove intermediate object of failed create reparse call
ab50485ea1b4 smb: Update comments about some reparse point tags
1600fe2d42a1 smb: client: stop flooding dmesg with automounts
f7a33d56e52f smb: client: stop flooding dmesg on failed session setups
e1b72ef3ba03 cifs: Check for UTF-16 null codepoint in SFU symlink
target location
9717d5343849 Merge tag 'v6.12-rc-ksmbd-server-fixes' of
git://git.samba.org/ksmbd

On Sun, Sep 29, 2024 at 9:04 AM Pali Rohár <pali@kernel.org> wrote:
>
> If CREATE was successful but SMB2_OP_SET_REPARSE failed then remove the
> intermediate object created by CREATE. Otherwise empty object stay on the
> server when reparse call failed.
>
> This ensures that if the creating of special files is unsupported by the
> server then no empty file stay on the server as a result of unsupported
> operation.
>
> Fixes: 102466f303ff ("smb: client: allow creating special files via reparse points")
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
> Changes in v2:
> * Increase out_buftype[] and out_iov[] members from 2 to 4 as required by smb2_compound_op
> * Call free_rsp_buf() for all members of out_buftype[]/out_iov[]
>
> I would like if you double check this smb2_compound_op() usage if there
> is not some other memory issue. As V1 contained both memory leak and
> buffer overflow (smb2_compound_op wrote out of those two arrays).
>
> ---
>  fs/smb/client/smb2inode.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> index 11a1c53c64e0..6e69a3b98be3 100644
> --- a/fs/smb/client/smb2inode.c
> +++ b/fs/smb/client/smb2inode.c
> @@ -1205,9 +1205,12 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
>         struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
>         struct cifsFileInfo *cfile;
>         struct inode *new = NULL;
> +       int out_buftype[4] = {};
> +       struct kvec out_iov[4] = {};
>         struct kvec in_iov[2];
>         int cmds[2];
>         int rc;
> +       int i;
>
>         oparms = CIFS_OPARMS(cifs_sb, tcon, full_path,
>                              SYNCHRONIZE | DELETE |
> @@ -1228,7 +1231,7 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
>                 cmds[1] = SMB2_OP_POSIX_QUERY_INFO;
>                 cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
>                 rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
> -                                     in_iov, cmds, 2, cfile, NULL, NULL, NULL);
> +                                     in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
>                 if (!rc) {
>                         rc = smb311_posix_get_inode_info(&new, full_path,
>                                                          data, sb, xid);
> @@ -1237,12 +1240,27 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
>                 cmds[1] = SMB2_OP_QUERY_INFO;
>                 cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
>                 rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
> -                                     in_iov, cmds, 2, cfile, NULL, NULL, NULL);
> +                                     in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
>                 if (!rc) {
>                         rc = cifs_get_inode_info(&new, full_path,
>                                                  data, sb, xid, NULL);
>                 }
>         }
> +
> +       if (rc) {
> +               /*
> +                * If CREATE was successful but SMB2_OP_SET_REPARSE failed then
> +                * remove the intermediate object created by CREATE. Otherwise
> +                * empty object stay on the server when reparse call failed.
> +                */
> +               if (((struct smb2_hdr *)out_iov[0].iov_base)->Status == STATUS_SUCCESS &&
> +                   ((struct smb2_hdr *)out_iov[1].iov_base)->Status != STATUS_SUCCESS)
> +                       smb2_unlink(xid, tcon, full_path, cifs_sb, NULL);
> +       }
> +
> +       for (i = 0; i < ARRAY_SIZE(out_buftype); i++)
> +               free_rsp_buf(out_buftype[i], out_iov[i].iov_base);
> +
>         return rc ? ERR_PTR(rc) : new;
>  }
>
> --
> 2.20.1
>
>


-- 
Thanks,

Steve

Re: [PATCH 2/8] cifs: Remove intermediate object of failed create reparse call

[Paulo Alcantara]

Pali Rohár <pali@kernel.org> writes:

> If CREATE was successful but SMB2_OP_SET_REPARSE failed then remove the
> intermediate object created by CREATE. Otherwise empty object stay on the
> server when reparse call failed.
>
> This ensures that if the creating of special files is unsupported by the
> server then no empty file stay on the server as a result of unsupported
> operation.
>
> Fixes: 102466f303ff ("smb: client: allow creating special files via reparse points")
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  fs/smb/client/smb2inode.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> index 11a1c53c64e0..af42f44bdcf4 100644
> --- a/fs/smb/client/smb2inode.c
> +++ b/fs/smb/client/smb2inode.c
> @@ -1205,6 +1205,8 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
>  	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
>  	struct cifsFileInfo *cfile;
>  	struct inode *new = NULL;
> +	int out_buftype[2] = {};
> +	struct kvec out_iov[2];
>  	struct kvec in_iov[2];
>  	int cmds[2];
>  	int rc;
> @@ -1228,7 +1230,7 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
>  		cmds[1] = SMB2_OP_POSIX_QUERY_INFO;
>  		cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
>  		rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
> -				      in_iov, cmds, 2, cfile, NULL, NULL, NULL);
> +				      in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
>  		if (!rc) {
>  			rc = smb311_posix_get_inode_info(&new, full_path,
>  							 data, sb, xid);
> @@ -1237,12 +1239,27 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
>  		cmds[1] = SMB2_OP_QUERY_INFO;
>  		cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
>  		rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
> -				      in_iov, cmds, 2, cfile, NULL, NULL, NULL);
> +				      in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
>  		if (!rc) {
>  			rc = cifs_get_inode_info(&new, full_path,
>  						 data, sb, xid, NULL);
>  		}
>  	}
> +
> +	if (rc) {
> +		/*
> +		 * If CREATE was successful but SMB2_OP_SET_REPARSE failed then
> +		 * remove the intermediate object created by CREATE. Otherwise
> +		 * empty object stay on the server when reparse call failed.
> +		 */
> +		if (((struct smb2_hdr *)out_iov[0].iov_base)->Status == STATUS_SUCCESS &&
> +		    ((struct smb2_hdr *)out_iov[1].iov_base)->Status != STATUS_SUCCESS)
> +			smb2_unlink(xid, tcon, full_path, cifs_sb, NULL);
> +	}

You should handle the case where ->iov_base is NULL or out_buftype ==
CIFS_NO_BUFFER, otherwise you'll end up with a NULL ptr deref.

Re: [PATCH 2/8] cifs: Remove intermediate object of failed create reparse call

[Pali Rohár]

On Monday 30 September 2024 12:25:27 Paulo Alcantara wrote:
> Pali Rohár <pali@kernel.org> writes:
> 
> > If CREATE was successful but SMB2_OP_SET_REPARSE failed then remove the
> > intermediate object created by CREATE. Otherwise empty object stay on the
> > server when reparse call failed.
> >
> > This ensures that if the creating of special files is unsupported by the
> > server then no empty file stay on the server as a result of unsupported
> > operation.
> >
> > Fixes: 102466f303ff ("smb: client: allow creating special files via reparse points")
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  fs/smb/client/smb2inode.c | 21 +++++++++++++++++++--
> >  1 file changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> > index 11a1c53c64e0..af42f44bdcf4 100644
> > --- a/fs/smb/client/smb2inode.c
> > +++ b/fs/smb/client/smb2inode.c
> > @@ -1205,6 +1205,8 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
> >  	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
> >  	struct cifsFileInfo *cfile;
> >  	struct inode *new = NULL;
> > +	int out_buftype[2] = {};
> > +	struct kvec out_iov[2];
> >  	struct kvec in_iov[2];
> >  	int cmds[2];
> >  	int rc;
> > @@ -1228,7 +1230,7 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
> >  		cmds[1] = SMB2_OP_POSIX_QUERY_INFO;
> >  		cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
> >  		rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
> > -				      in_iov, cmds, 2, cfile, NULL, NULL, NULL);
> > +				      in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
> >  		if (!rc) {
> >  			rc = smb311_posix_get_inode_info(&new, full_path,
> >  							 data, sb, xid);
> > @@ -1237,12 +1239,27 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
> >  		cmds[1] = SMB2_OP_QUERY_INFO;
> >  		cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
> >  		rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
> > -				      in_iov, cmds, 2, cfile, NULL, NULL, NULL);
> > +				      in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
> >  		if (!rc) {
> >  			rc = cifs_get_inode_info(&new, full_path,
> >  						 data, sb, xid, NULL);
> >  		}
> >  	}
> > +
> > +	if (rc) {
> > +		/*
> > +		 * If CREATE was successful but SMB2_OP_SET_REPARSE failed then
> > +		 * remove the intermediate object created by CREATE. Otherwise
> > +		 * empty object stay on the server when reparse call failed.
> > +		 */
> > +		if (((struct smb2_hdr *)out_iov[0].iov_base)->Status == STATUS_SUCCESS &&
> > +		    ((struct smb2_hdr *)out_iov[1].iov_base)->Status != STATUS_SUCCESS)
> > +			smb2_unlink(xid, tcon, full_path, cifs_sb, NULL);
> > +	}
> 
> You should handle the case where ->iov_base is NULL or out_buftype ==
> CIFS_NO_BUFFER, otherwise you'll end up with a NULL ptr deref.

Ok, thanks for info! I will send v3 with those checks.

Anyway, what does it mean if iov_base stay NULL or out_buftype is
CIFS_NO_BUFFER? Does it mean that the server has not returned reply for
that command?

I guess that it should be treated as unsuccessful result.

Re: [PATCH 2/8] cifs: Remove intermediate object of failed create reparse call

[Paulo Alcantara]

Pali Rohár <pali@kernel.org> writes:

> On Monday 30 September 2024 12:25:27 Paulo Alcantara wrote:
>> Pali Rohár <pali@kernel.org> writes:
>> 
>> > If CREATE was successful but SMB2_OP_SET_REPARSE failed then remove the
>> > intermediate object created by CREATE. Otherwise empty object stay on the
>> > server when reparse call failed.
>> >
>> > This ensures that if the creating of special files is unsupported by the
>> > server then no empty file stay on the server as a result of unsupported
>> > operation.
>> >
>> > Fixes: 102466f303ff ("smb: client: allow creating special files via reparse points")
>> > Signed-off-by: Pali Rohár <pali@kernel.org>
>> > ---
>> >  fs/smb/client/smb2inode.c | 21 +++++++++++++++++++--
>> >  1 file changed, 19 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
>> > index 11a1c53c64e0..af42f44bdcf4 100644
>> > --- a/fs/smb/client/smb2inode.c
>> > +++ b/fs/smb/client/smb2inode.c
>> > @@ -1205,6 +1205,8 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
>> >  	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
>> >  	struct cifsFileInfo *cfile;
>> >  	struct inode *new = NULL;
>> > +	int out_buftype[2] = {};
>> > +	struct kvec out_iov[2];
>> >  	struct kvec in_iov[2];
>> >  	int cmds[2];
>> >  	int rc;
>> > @@ -1228,7 +1230,7 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
>> >  		cmds[1] = SMB2_OP_POSIX_QUERY_INFO;
>> >  		cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
>> >  		rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
>> > -				      in_iov, cmds, 2, cfile, NULL, NULL, NULL);
>> > +				      in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
>> >  		if (!rc) {
>> >  			rc = smb311_posix_get_inode_info(&new, full_path,
>> >  							 data, sb, xid);
>> > @@ -1237,12 +1239,27 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
>> >  		cmds[1] = SMB2_OP_QUERY_INFO;
>> >  		cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
>> >  		rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
>> > -				      in_iov, cmds, 2, cfile, NULL, NULL, NULL);
>> > +				      in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
>> >  		if (!rc) {
>> >  			rc = cifs_get_inode_info(&new, full_path,
>> >  						 data, sb, xid, NULL);
>> >  		}
>> >  	}
>> > +
>> > +	if (rc) {
>> > +		/*
>> > +		 * If CREATE was successful but SMB2_OP_SET_REPARSE failed then
>> > +		 * remove the intermediate object created by CREATE. Otherwise
>> > +		 * empty object stay on the server when reparse call failed.
>> > +		 */
>> > +		if (((struct smb2_hdr *)out_iov[0].iov_base)->Status == STATUS_SUCCESS &&
>> > +		    ((struct smb2_hdr *)out_iov[1].iov_base)->Status != STATUS_SUCCESS)
>> > +			smb2_unlink(xid, tcon, full_path, cifs_sb, NULL);
>> > +	}
>> 
>> You should handle the case where ->iov_base is NULL or out_buftype ==
>> CIFS_NO_BUFFER, otherwise you'll end up with a NULL ptr deref.
>
> Ok, thanks for info! I will send v3 with those checks.
>
> Anyway, what does it mean if iov_base stay NULL or out_buftype is
> CIFS_NO_BUFFER? Does it mean that the server has not returned reply for
> that command?

Not necessarily.  Just consider a simple case where smb2_compound_op()
would fail to allocate memory for @vars, then it would return -ENOMEM
and you would end up dereferencing ->iov_base which is still NULL.  That
is, compound_send_recv() wasn't called and then no response buffers were
set yet.

[PATCH v3] cifs: Remove intermediate object of failed create reparse call

[Pali Rohár]

If CREATE was successful but SMB2_OP_SET_REPARSE failed then remove the
intermediate object created by CREATE. Otherwise empty object stay on the
server when reparse call failed.

This ensures that if the creating of special files is unsupported by the
server then no empty file stay on the server as a result of unsupported
operation.

Fixes: 102466f303ff ("smb: client: allow creating special files via reparse points")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
Changes in v3:
* Check if iov_base and out_buftype are valid before derefrencing iov_base
Changes in v2:
* Increase out_buftype[] and out_iov[] members from 2 to 4 as required by smb2_compound_op
* Call free_rsp_buf() for all members of out_buftype[]/out_iov[]
---
 fs/smb/client/smb2inode.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 11a1c53c64e0..a6dab60e2c01 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -1205,9 +1205,12 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
 	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
 	struct cifsFileInfo *cfile;
 	struct inode *new = NULL;
+	int out_buftype[4] = {};
+	struct kvec out_iov[4] = {};
 	struct kvec in_iov[2];
 	int cmds[2];
 	int rc;
+	int i;
 
 	oparms = CIFS_OPARMS(cifs_sb, tcon, full_path,
 			     SYNCHRONIZE | DELETE |
@@ -1228,7 +1231,7 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
 		cmds[1] = SMB2_OP_POSIX_QUERY_INFO;
 		cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
 		rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
-				      in_iov, cmds, 2, cfile, NULL, NULL, NULL);
+				      in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
 		if (!rc) {
 			rc = smb311_posix_get_inode_info(&new, full_path,
 							 data, sb, xid);
@@ -1237,12 +1240,29 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
 		cmds[1] = SMB2_OP_QUERY_INFO;
 		cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
 		rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
-				      in_iov, cmds, 2, cfile, NULL, NULL, NULL);
+				      in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
 		if (!rc) {
 			rc = cifs_get_inode_info(&new, full_path,
 						 data, sb, xid, NULL);
 		}
 	}
+
+
+	/*
+	 * If CREATE was successful but SMB2_OP_SET_REPARSE failed then
+	 * remove the intermediate object created by CREATE. Otherwise
+	 * empty object stay on the server when reparse call failed.
+	 */
+	if (rc &&
+	    out_iov[0].iov_base != NULL && out_buftype[0] != CIFS_NO_BUFFER &&
+	    ((struct smb2_hdr *)out_iov[0].iov_base)->Status == STATUS_SUCCESS &&
+	    (out_iov[1].iov_base == NULL || out_buftype[1] == CIFS_NO_BUFFER ||
+	     ((struct smb2_hdr *)out_iov[1].iov_base)->Status != STATUS_SUCCESS))
+		smb2_unlink(xid, tcon, full_path, cifs_sb, NULL);
+
+	for (i = 0; i < ARRAY_SIZE(out_buftype); i++)
+		free_rsp_buf(out_buftype[i], out_iov[i].iov_base);
+
 	return rc ? ERR_PTR(rc) : new;
 }
 
-- 
2.20.1

Re: [PATCH v3] cifs: Remove intermediate object of failed create reparse call

[Steve French]

tentatively merged into cifs-2.6.git pending review and testing

On Mon, Sep 30, 2024 at 3:28 PM Pali Rohár <pali@kernel.org> wrote:
>
> If CREATE was successful but SMB2_OP_SET_REPARSE failed then remove the
> intermediate object created by CREATE. Otherwise empty object stay on the
> server when reparse call failed.
>
> This ensures that if the creating of special files is unsupported by the
> server then no empty file stay on the server as a result of unsupported
> operation.
>
> Fixes: 102466f303ff ("smb: client: allow creating special files via reparse points")
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
> Changes in v3:
> * Check if iov_base and out_buftype are valid before derefrencing iov_base
> Changes in v2:
> * Increase out_buftype[] and out_iov[] members from 2 to 4 as required by smb2_compound_op
> * Call free_rsp_buf() for all members of out_buftype[]/out_iov[]
> ---
>  fs/smb/client/smb2inode.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> index 11a1c53c64e0..a6dab60e2c01 100644
> --- a/fs/smb/client/smb2inode.c
> +++ b/fs/smb/client/smb2inode.c
> @@ -1205,9 +1205,12 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
>         struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
>         struct cifsFileInfo *cfile;
>         struct inode *new = NULL;
> +       int out_buftype[4] = {};
> +       struct kvec out_iov[4] = {};
>         struct kvec in_iov[2];
>         int cmds[2];
>         int rc;
> +       int i;
>
>         oparms = CIFS_OPARMS(cifs_sb, tcon, full_path,
>                              SYNCHRONIZE | DELETE |
> @@ -1228,7 +1231,7 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
>                 cmds[1] = SMB2_OP_POSIX_QUERY_INFO;
>                 cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
>                 rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
> -                                     in_iov, cmds, 2, cfile, NULL, NULL, NULL);
> +                                     in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
>                 if (!rc) {
>                         rc = smb311_posix_get_inode_info(&new, full_path,
>                                                          data, sb, xid);
> @@ -1237,12 +1240,29 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data,
>                 cmds[1] = SMB2_OP_QUERY_INFO;
>                 cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
>                 rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, &oparms,
> -                                     in_iov, cmds, 2, cfile, NULL, NULL, NULL);
> +                                     in_iov, cmds, 2, cfile, out_iov, out_buftype, NULL);
>                 if (!rc) {
>                         rc = cifs_get_inode_info(&new, full_path,
>                                                  data, sb, xid, NULL);
>                 }
>         }
> +
> +
> +       /*
> +        * If CREATE was successful but SMB2_OP_SET_REPARSE failed then
> +        * remove the intermediate object created by CREATE. Otherwise
> +        * empty object stay on the server when reparse call failed.
> +        */
> +       if (rc &&
> +           out_iov[0].iov_base != NULL && out_buftype[0] != CIFS_NO_BUFFER &&
> +           ((struct smb2_hdr *)out_iov[0].iov_base)->Status == STATUS_SUCCESS &&
> +           (out_iov[1].iov_base == NULL || out_buftype[1] == CIFS_NO_BUFFER ||
> +            ((struct smb2_hdr *)out_iov[1].iov_base)->Status != STATUS_SUCCESS))
> +               smb2_unlink(xid, tcon, full_path, cifs_sb, NULL);
> +
> +       for (i = 0; i < ARRAY_SIZE(out_buftype); i++)
> +               free_rsp_buf(out_buftype[i], out_iov[i].iov_base);
> +
>         return rc ? ERR_PTR(rc) : new;
>  }
>
> --
> 2.20.1
>
>


-- 
Thanks,

Steve

[PATCH 3/8] cifs: Fix parsing NFS-style char/block devices

[Pali Rohár]

Linux SMB client currently parses NFS-style char and block devices
incorrectly. It reads major number from location of minor and major from
location of minor.

Per MS-FSCC 2.1.2.6 NFS_SPECFILE_CHR and NFS_SPECFILE_BLK DataBuffer's
field contains two 32-bit integers that represent major and minor device
numbers.

So the first one 32-bit integer in DataBuffer is major number and second
one in DataBuffer is minor number. Microsoft Windows NFS server reads them
in this order too.

This issue was introduced in commit 45e724022e27 ("smb: client: set correct
file type from NFS reparse points") and probably because in commit message
was test of char and block devices with same major and minor numbers. So
swapped major and minor numbers were not spotted.

Fix this problem in Linux SMB client by reading major and minor numbers
from correct position of DataBuffer.

This change fixes interoperability of char and block devices on Windows
share which is exported over both SMB and NFS protocols.

Fixes: 45e724022e27 ("smb: client: set correct file type from NFS reparse points")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/reparse.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/reparse.h b/fs/smb/client/reparse.h
index 2c0644bc4e65..790360f8a53b 100644
--- a/fs/smb/client/reparse.h
+++ b/fs/smb/client/reparse.h
@@ -20,9 +20,12 @@
 
 static inline dev_t reparse_nfs_mkdev(struct reparse_posix_data *buf)
 {
-	u64 v = le64_to_cpu(*(__le64 *)buf->DataBuffer);
+	u32 major, minor;
 
-	return MKDEV(v >> 32, v & 0xffffffff);
+	major = le32_to_cpu(((__le32 *)buf->DataBuffer)[0]);
+	minor = le32_to_cpu(((__le32 *)buf->DataBuffer)[1]);
+
+	return MKDEV(major, minor);
 }
 
 static inline dev_t wsl_mkdev(void *ptr)
-- 
2.20.1

[PATCH 4/8] cifs: Fix creating NFS-style char/block devices

[Pali Rohár]

Linux SMB client currently creates NFS-style char and block devices with
swapped major and minor numbers.

Per MS-FSCC 2.1.2.6 NFS_SPECFILE_CHR and NFS_SPECFILE_BLK DataBuffer's
field contains two 32-bit integers that represent major and minor device
numbers.

So the first one 32-bit integer in DataBuffer is major number and second
one in DataBuffer is minor number. Microsoft Windows NFS server reads them
in this order too.

But Linux CIFS client creates new reparse point DataBuffer with minor
number first and major number second.

Fix this problem in Linux SMB client and puts major and minor number in
the correct order into DataBuffer.

This change fixes interoperability of char and block devices on Windows
share which is exported over both SMB and NFS protocols.

Fixes: 102466f303ff ("smb: client: allow creating special files via reparse points")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/reparse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index 48c27581ec51..63984796721a 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -108,8 +108,8 @@ static int nfs_set_reparse_buf(struct reparse_posix_data *buf,
 	buf->InodeType = cpu_to_le64(type);
 	buf->ReparseDataLength = cpu_to_le16(len + dlen -
 					     sizeof(struct reparse_data_buffer));
-	*(__le64 *)buf->DataBuffer = cpu_to_le64(((u64)MAJOR(dev) << 32) |
-						 MINOR(dev));
+	*(__le64 *)buf->DataBuffer = cpu_to_le64(((u64)MINOR(dev) << 32) |
+						 MAJOR(dev));
 	iov->iov_base = buf;
 	iov->iov_len = len + dlen;
 	return 0;
-- 
2.20.1

Re: [PATCH 4/8] cifs: Fix creating NFS-style char/block devices

[Steve French]

Looks like a duplicate of Paulo's earlier already merged patch, so
will skip this one.  Reviewing the others in the series now.

commit a9de67336a4aa3ff2e706ba023fb5f7ff681a954
Author: Paulo Alcantara <pc@manguebit.com>
Date:   Wed Sep 18 21:53:35 2024 -0300

    smb: client: set correct device number on nfs reparse points

    Fix major and minor numbers set on special files created with NFS
    reparse points.

On Sat, Sep 28, 2024 at 5:02 PM Pali Rohár <pali@kernel.org> wrote:
>
> Linux SMB client currently creates NFS-style char and block devices with
> swapped major and minor numbers.
>
> Per MS-FSCC 2.1.2.6 NFS_SPECFILE_CHR and NFS_SPECFILE_BLK DataBuffer's
> field contains two 32-bit integers that represent major and minor device
> numbers.
>
> So the first one 32-bit integer in DataBuffer is major number and second
> one in DataBuffer is minor number. Microsoft Windows NFS server reads them
> in this order too.
>
> But Linux CIFS client creates new reparse point DataBuffer with minor
> number first and major number second.
>
> Fix this problem in Linux SMB client and puts major and minor number in
> the correct order into DataBuffer.
>
> This change fixes interoperability of char and block devices on Windows
> share which is exported over both SMB and NFS protocols.
>
> Fixes: 102466f303ff ("smb: client: allow creating special files via reparse points")
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  fs/smb/client/reparse.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
> index 48c27581ec51..63984796721a 100644
> --- a/fs/smb/client/reparse.c
> +++ b/fs/smb/client/reparse.c
> @@ -108,8 +108,8 @@ static int nfs_set_reparse_buf(struct reparse_posix_data *buf,
>         buf->InodeType = cpu_to_le64(type);
>         buf->ReparseDataLength = cpu_to_le16(len + dlen -
>                                              sizeof(struct reparse_data_buffer));
> -       *(__le64 *)buf->DataBuffer = cpu_to_le64(((u64)MAJOR(dev) << 32) |
> -                                                MINOR(dev));
> +       *(__le64 *)buf->DataBuffer = cpu_to_le64(((u64)MINOR(dev) << 32) |
> +                                                MAJOR(dev));
>         iov->iov_base = buf;
>         iov->iov_len = len + dlen;
>         return 0;
> --
> 2.20.1
>
>


-- 
Thanks,

Steve

Re: [PATCH 4/8] cifs: Fix creating NFS-style char/block devices

[Pali Rohár]

Ops, sorry for that. I just let my work branch on v6.11-rc7 and here
this change was not yet. But it is funny that we have hit this problem
independently in nearly same time.

On Saturday 28 September 2024 19:18:26 Steve French wrote:
> Looks like a duplicate of Paulo's earlier already merged patch, so
> will skip this one.  Reviewing the others in the series now.
> 
> commit a9de67336a4aa3ff2e706ba023fb5f7ff681a954
> Author: Paulo Alcantara <pc@manguebit.com>
> Date:   Wed Sep 18 21:53:35 2024 -0300
> 
>     smb: client: set correct device number on nfs reparse points
> 
>     Fix major and minor numbers set on special files created with NFS
>     reparse points.
> 
> On Sat, Sep 28, 2024 at 5:02 PM Pali Rohár <pali@kernel.org> wrote:
> >
> > Linux SMB client currently creates NFS-style char and block devices with
> > swapped major and minor numbers.
> >
> > Per MS-FSCC 2.1.2.6 NFS_SPECFILE_CHR and NFS_SPECFILE_BLK DataBuffer's
> > field contains two 32-bit integers that represent major and minor device
> > numbers.
> >
> > So the first one 32-bit integer in DataBuffer is major number and second
> > one in DataBuffer is minor number. Microsoft Windows NFS server reads them
> > in this order too.
> >
> > But Linux CIFS client creates new reparse point DataBuffer with minor
> > number first and major number second.
> >
> > Fix this problem in Linux SMB client and puts major and minor number in
> > the correct order into DataBuffer.
> >
> > This change fixes interoperability of char and block devices on Windows
> > share which is exported over both SMB and NFS protocols.
> >
> > Fixes: 102466f303ff ("smb: client: allow creating special files via reparse points")
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  fs/smb/client/reparse.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
> > index 48c27581ec51..63984796721a 100644
> > --- a/fs/smb/client/reparse.c
> > +++ b/fs/smb/client/reparse.c
> > @@ -108,8 +108,8 @@ static int nfs_set_reparse_buf(struct reparse_posix_data *buf,
> >         buf->InodeType = cpu_to_le64(type);
> >         buf->ReparseDataLength = cpu_to_le16(len + dlen -
> >                                              sizeof(struct reparse_data_buffer));
> > -       *(__le64 *)buf->DataBuffer = cpu_to_le64(((u64)MAJOR(dev) << 32) |
> > -                                                MINOR(dev));
> > +       *(__le64 *)buf->DataBuffer = cpu_to_le64(((u64)MINOR(dev) << 32) |
> > +                                                MAJOR(dev));
> >         iov->iov_base = buf;
> >         iov->iov_len = len + dlen;
> >         return 0;
> > --
> > 2.20.1
> >
> >
> 
> 
> -- 
> Thanks,
> 
> Steve

Re: [PATCH 4/8] cifs: Fix creating NFS-style char/block devices

[Pali Rohár]

I guess that patch 3 can be dropped too as Paulo fixed it recently in:
https://git.samba.org/sfrench/?p=sfrench/cifs-2.6.git;a=commit;h=663f295e35594f4c2584fc68c28546b747b637cd

On Saturday 28 September 2024 19:47:52 Steve French wrote:
> Patch 3 may need to be rebased, iirc patch 3 didn't merge but 1 and 2 did
> 
> On Sat, Sep 28, 2024, 7:44 PM Pali Rohár <pali@kernel.org> wrote:
> 
> > Ops, sorry for that. I just let my work branch on v6.11-rc7 and here
> > this change was not yet. But it is funny that we have hit this problem
> > independently in nearly same time.
> >
> > On Saturday 28 September 2024 19:18:26 Steve French wrote:
> > > Looks like a duplicate of Paulo's earlier already merged patch, so
> > > will skip this one.  Reviewing the others in the series now.
> > >
> > > commit a9de67336a4aa3ff2e706ba023fb5f7ff681a954
> > > Author: Paulo Alcantara <pc@manguebit.com>
> > > Date:   Wed Sep 18 21:53:35 2024 -0300
> > >
> > >     smb: client: set correct device number on nfs reparse points
> > >
> > >     Fix major and minor numbers set on special files created with NFS
> > >     reparse points.
> > >
> > > On Sat, Sep 28, 2024 at 5:02 PM Pali Rohár <pali@kernel.org> wrote:
> > > >
> > > > Linux SMB client currently creates NFS-style char and block devices
> > with
> > > > swapped major and minor numbers.
> > > >
> > > > Per MS-FSCC 2.1.2.6 NFS_SPECFILE_CHR and NFS_SPECFILE_BLK DataBuffer's
> > > > field contains two 32-bit integers that represent major and minor
> > device
> > > > numbers.
> > > >
> > > > So the first one 32-bit integer in DataBuffer is major number and
> > second
> > > > one in DataBuffer is minor number. Microsoft Windows NFS server reads
> > them
> > > > in this order too.
> > > >
> > > > But Linux CIFS client creates new reparse point DataBuffer with minor
> > > > number first and major number second.
> > > >
> > > > Fix this problem in Linux SMB client and puts major and minor number in
> > > > the correct order into DataBuffer.
> > > >
> > > > This change fixes interoperability of char and block devices on Windows
> > > > share which is exported over both SMB and NFS protocols.
> > > >
> > > > Fixes: 102466f303ff ("smb: client: allow creating special files via
> > reparse points")
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > ---
> > > >  fs/smb/client/reparse.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
> > > > index 48c27581ec51..63984796721a 100644
> > > > --- a/fs/smb/client/reparse.c
> > > > +++ b/fs/smb/client/reparse.c
> > > > @@ -108,8 +108,8 @@ static int nfs_set_reparse_buf(struct
> > reparse_posix_data *buf,
> > > >         buf->InodeType = cpu_to_le64(type);
> > > >         buf->ReparseDataLength = cpu_to_le16(len + dlen -
> > > >                                              sizeof(struct
> > reparse_data_buffer));
> > > > -       *(__le64 *)buf->DataBuffer = cpu_to_le64(((u64)MAJOR(dev) <<
> > 32) |
> > > > -                                                MINOR(dev));
> > > > +       *(__le64 *)buf->DataBuffer = cpu_to_le64(((u64)MINOR(dev) <<
> > 32) |
> > > > +                                                MAJOR(dev));
> > > >         iov->iov_base = buf;
> > > >         iov->iov_len = len + dlen;
> > > >         return 0;
> > > > --
> > > > 2.20.1
> > > >
> > > >
> > >
> > >
> > > --
> > > Thanks,
> > >
> > > Steve
> >

[PATCH 5/8] cifs: Fix buffer overflow when parsing NFS reparse points

[Pali Rohár]

ReparseDataLength is sum of the InodeType size and DataBuffer size.
So to get DataBuffer size it is needed to subtract InodeType's size from
ReparseDataLength.

Function cifs_strndup_from_utf16() is currentlly accessing buf->DataBuffer
at position after the end of the buffer because it does not subtract
InodeType size from the length. Fix this problem and correctly subtract
variable len.

Member InodeType is present only when reparse buffer is large enough. Check
for ReparseDataLength before accessing InodeType to prevent another invalid
memory access.

Major and minor rdev values are present also only when reparse buffer is
large enough. Check for reparse buffer size in reparse_nfs_mkdev().

Fixes: d5ecebc4900d ("smb3: Allow query of symlinks stored as reparse points")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/reparse.c | 11 ++++++++++-
 fs/smb/client/reparse.h |  3 +++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index 63984796721a..af32a7dbca4b 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -320,9 +320,16 @@ static int parse_reparse_posix(struct reparse_posix_data *buf,
 	unsigned int len;
 	u64 type;
 
+	len = le16_to_cpu(buf->ReparseDataLength);
+	if (len < sizeof(buf->InodeType)) {
+		cifs_dbg(VFS, "srv returned malformed nfs buffer\n");
+		return -EIO;
+	}
+
+	len -= sizeof(buf->InodeType);
+
 	switch ((type = le64_to_cpu(buf->InodeType))) {
 	case NFS_SPECFILE_LNK:
-		len = le16_to_cpu(buf->ReparseDataLength);
 		data->symlink_target = cifs_strndup_from_utf16(buf->DataBuffer,
 							       len, true,
 							       cifs_sb->local_nls);
@@ -482,6 +489,8 @@ bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
 	u32 tag = data->reparse.tag;
 
 	if (tag == IO_REPARSE_TAG_NFS && buf) {
+		if (le16_to_cpu(buf->ReparseDataLength) < sizeof(buf->InodeType))
+			return false;
 		switch (le64_to_cpu(buf->InodeType)) {
 		case NFS_SPECFILE_CHR:
 			fattr->cf_mode |= S_IFCHR;
diff --git a/fs/smb/client/reparse.h b/fs/smb/client/reparse.h
index 790360f8a53b..5be54878265e 100644
--- a/fs/smb/client/reparse.h
+++ b/fs/smb/client/reparse.h
@@ -22,6 +22,9 @@ static inline dev_t reparse_nfs_mkdev(struct reparse_posix_data *buf)
 {
 	u32 major, minor;
 
+	if (le16_to_cpu(buf->ReparseDataLength) != sizeof(buf->InodeType) + 2*sizeof(__le32))
+		return 0;
+
 	major = le32_to_cpu(((__le32 *)buf->DataBuffer)[0]);
 	minor = le32_to_cpu(((__le32 *)buf->DataBuffer)[1]);
 
-- 
2.20.1

[PATCH v2] cifs: Fix buffer overflow when parsing NFS reparse points

[Pali Rohár]

ReparseDataLength is sum of the InodeType size and DataBuffer size.
So to get DataBuffer size it is needed to subtract InodeType's size from
ReparseDataLength.

Function cifs_strndup_from_utf16() is currentlly accessing buf->DataBuffer
at position after the end of the buffer because it does not subtract
InodeType size from the length. Fix this problem and correctly subtract
variable len.

Member InodeType is present only when reparse buffer is large enough. Check
for ReparseDataLength before accessing InodeType to prevent another invalid
memory access.

Major and minor rdev values are present also only when reparse buffer is
large enough. Check for reparse buffer size before calling reparse_mkdev().

Fixes: d5ecebc4900d ("smb3: Allow query of symlinks stored as reparse points")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
Changes in v2:
* Rebased on top of cifs-2.6 for-next (e5113352e8739469445b2211e809ac937f132aa9)
---
 fs/smb/client/reparse.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index 0ad94d1cb57c..11cebb320012 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -320,9 +320,16 @@ static int parse_reparse_posix(struct reparse_posix_data *buf,
 	unsigned int len;
 	u64 type;
 
+	len = le16_to_cpu(buf->ReparseDataLength);
+	if (len < sizeof(buf->InodeType)) {
+		cifs_dbg(VFS, "srv returned malformed nfs buffer\n");
+		return -EIO;
+	}
+
+	len -= sizeof(buf->InodeType);
+
 	switch ((type = le64_to_cpu(buf->InodeType))) {
 	case NFS_SPECFILE_LNK:
-		len = le16_to_cpu(buf->ReparseDataLength);
 		data->symlink_target = cifs_strndup_from_utf16(buf->DataBuffer,
 							       len, true,
 							       cifs_sb->local_nls);
@@ -481,12 +488,18 @@ bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
 	u32 tag = data->reparse.tag;
 
 	if (tag == IO_REPARSE_TAG_NFS && buf) {
+		if (le16_to_cpu(buf->ReparseDataLength) < sizeof(buf->InodeType))
+			return false;
 		switch (le64_to_cpu(buf->InodeType)) {
 		case NFS_SPECFILE_CHR:
+			if (le16_to_cpu(buf->ReparseDataLength) != sizeof(buf->InodeType) + 8)
+				return false;
 			fattr->cf_mode |= S_IFCHR;
 			fattr->cf_rdev = reparse_mkdev(buf->DataBuffer);
 			break;
 		case NFS_SPECFILE_BLK:
+			if (le16_to_cpu(buf->ReparseDataLength) != sizeof(buf->InodeType) + 8)
+				return false;
 			fattr->cf_mode |= S_IFBLK;
 			fattr->cf_rdev = reparse_mkdev(buf->DataBuffer);
 			break;
-- 
2.20.1

[PATCH 6/8] cifs: Do not convert delimiter when parsing NFS-style symlinks

[Pali Rohár]

NFS-style symlinks have target location always stored in NFS/UNIX form
where backslash means the real UNIX backslash and not the SMB path
separator.

So do not mangle slash and backslash content of NFS-style symlink during
readlink() syscall as it is already in the correct Linux form.

This fixes interoperability of NFS-style symlinks with backslashes created
by Linux NFS3 client throw Windows NFS server and retrieved by Linux SMB
client throw Windows SMB server, where both Windows servers exports the
same directory.

Fixes: d5ecebc4900d ("smb3: Allow query of symlinks stored as reparse points")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/reparse.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index af32a7dbca4b..e3cf7ae516cb 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -335,7 +335,6 @@ static int parse_reparse_posix(struct reparse_posix_data *buf,
 							       cifs_sb->local_nls);
 		if (!data->symlink_target)
 			return -ENOMEM;
-		convert_delimiter(data->symlink_target, '/');
 		cifs_dbg(FYI, "%s: target path: %s\n",
 			 __func__, data->symlink_target);
 		break;
-- 
2.20.1

[PATCH 7/8] cifs: Validate content of NFS reparse point buffer

[Pali Rohár]

Symlink target location stored in DataBuffer is encoded in UTF-16. So check
that symlink DataBuffer length is non-zero and even number. And check that
DataBuffer does not contain UTF-16 null codepoint because Linux cannot
process symlink with null byte.

DataBuffer for char and block devices is 8 bytes long as it contains two
32-bit numbers (major and minor). Add check for this.

DataBuffer buffer for sockets and fifos zero-length. Add checks for this.

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

diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index e3cf7ae516cb..35e8f2e18530 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -330,6 +330,18 @@ static int parse_reparse_posix(struct reparse_posix_data *buf,
 
 	switch ((type = le64_to_cpu(buf->InodeType))) {
 	case NFS_SPECFILE_LNK:
+		if (len == 0 || (len % 2)) {
+			cifs_dbg(VFS, "srv returned malformed nfs symlink buffer\n");
+			return -EIO;
+		}
+		/*
+		 * Check that buffer does not contain UTF-16 null codepoint
+		 * because Linux cannot process symlink with null byte.
+		 */
+		if (UniStrnlen((wchar_t *)buf->DataBuffer, len/2) != len/2) {
+			cifs_dbg(VFS, "srv returned null byte in nfs symlink target location\n");
+			return -EIO;
+		}
 		data->symlink_target = cifs_strndup_from_utf16(buf->DataBuffer,
 							       len, true,
 							       cifs_sb->local_nls);
@@ -340,8 +352,19 @@ static int parse_reparse_posix(struct reparse_posix_data *buf,
 		break;
 	case NFS_SPECFILE_CHR:
 	case NFS_SPECFILE_BLK:
+		/* DataBuffer for block and char devices contains two 32-bit numbers */
+		if (len != 8) {
+			cifs_dbg(VFS, "srv returned malformed nfs buffer for type: 0x%llx\n", type);
+			return -EIO;
+		}
+		break;
 	case NFS_SPECFILE_FIFO:
 	case NFS_SPECFILE_SOCK:
+		/* DataBuffer for fifos and sockets is empty */
+		if (len != 0) {
+			cifs_dbg(VFS, "srv returned malformed nfs buffer for type: 0x%llx\n", type);
+			return -EIO;
+		}
 		break;
 	default:
 		cifs_dbg(VFS, "%s: unhandled inode type: 0x%llx\n",
-- 
2.20.1

[PATCH 8/8] cifs: Rename posix to nfs in parse_reparse_posix() and reparse_posix_data

[Pali Rohár]

This function parses NFS-style reparse points, which are used only by
Windows NFS server since Windows Server 2012 version. This style of special
files is not understood by Microsoft POSIX / Interix / SFU / SUA subsystems.

So make it clear that parse_reparse_posix() function and reparse_posix_data
structure are not POSIX general, but rather NFS specific.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifsglob.h |  2 +-
 fs/smb/client/cifspdu.h  |  2 +-
 fs/smb/client/reparse.c  | 14 +++++++-------
 fs/smb/client/reparse.h  |  2 +-
 fs/smb/common/smb2pdu.h  |  2 +-
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 9eae8649f90c..119537e98f77 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -223,7 +223,7 @@ struct cifs_open_info_data {
 		__u32 tag;
 		union {
 			struct reparse_data_buffer *buf;
-			struct reparse_posix_data *posix;
+			struct reparse_nfs_data *nfs;
 		};
 	} reparse;
 	struct {
diff --git a/fs/smb/client/cifspdu.h b/fs/smb/client/cifspdu.h
index c3b6263060b0..fefd7e5eb170 100644
--- a/fs/smb/client/cifspdu.h
+++ b/fs/smb/client/cifspdu.h
@@ -1506,7 +1506,7 @@ struct reparse_symlink_data {
 #define NFS_SPECFILE_BLK	0x00000000004B4C42
 #define NFS_SPECFILE_FIFO	0x000000004F464946
 #define NFS_SPECFILE_SOCK	0x000000004B434F53
-struct reparse_posix_data {
+struct reparse_nfs_data {
 	__le32	ReparseTag;
 	__le16	ReparseDataLength;
 	__u16	Reserved;
diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index 35e8f2e18530..a23ea2f78c09 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -81,7 +81,7 @@ int smb2_create_reparse_symlink(const unsigned int xid, struct inode *inode,
 	return rc;
 }
 
-static int nfs_set_reparse_buf(struct reparse_posix_data *buf,
+static int nfs_set_reparse_buf(struct reparse_nfs_data *buf,
 			       mode_t mode, dev_t dev,
 			       struct kvec *iov)
 {
@@ -120,20 +120,20 @@ static int mknod_nfs(unsigned int xid, struct inode *inode,
 		     const char *full_path, umode_t mode, dev_t dev)
 {
 	struct cifs_open_info_data data;
-	struct reparse_posix_data *p;
+	struct reparse_nfs_data *p;
 	struct inode *new;
 	struct kvec iov;
 	__u8 buf[sizeof(*p) + sizeof(__le64)];
 	int rc;
 
-	p = (struct reparse_posix_data *)buf;
+	p = (struct reparse_nfs_data *)buf;
 	rc = nfs_set_reparse_buf(p, mode, dev, &iov);
 	if (rc)
 		return rc;
 
 	data = (struct cifs_open_info_data) {
 		.reparse_point = true,
-		.reparse = { .tag = IO_REPARSE_TAG_NFS, .posix = p, },
+		.reparse = { .tag = IO_REPARSE_TAG_NFS, .nfs = p, },
 	};
 
 	new = smb2_get_reparse_inode(&data, inode->i_sb, xid,
@@ -313,7 +313,7 @@ int smb2_mknod_reparse(unsigned int xid, struct inode *inode,
 }
 
 /* See MS-FSCC 2.1.2.6 for the 'NFS' style reparse tags */
-static int parse_reparse_posix(struct reparse_posix_data *buf,
+static int parse_reparse_nfs(struct reparse_nfs_data *buf,
 			       struct cifs_sb_info *cifs_sb,
 			       struct cifs_open_info_data *data)
 {
@@ -414,7 +414,7 @@ int parse_reparse_point(struct reparse_data_buffer *buf,
 	/* See MS-FSCC 2.1.2 */
 	switch (le32_to_cpu(buf->ReparseTag)) {
 	case IO_REPARSE_TAG_NFS:
-		return parse_reparse_posix((struct reparse_posix_data *)buf,
+		return parse_reparse_nfs((struct reparse_nfs_data *)buf,
 					   cifs_sb, data);
 	case IO_REPARSE_TAG_SYMLINK:
 		return parse_reparse_symlink(
@@ -507,7 +507,7 @@ bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
 				 struct cifs_fattr *fattr,
 				 struct cifs_open_info_data *data)
 {
-	struct reparse_posix_data *buf = data->reparse.posix;
+	struct reparse_nfs_data *buf = data->reparse.nfs;
 	u32 tag = data->reparse.tag;
 
 	if (tag == IO_REPARSE_TAG_NFS && buf) {
diff --git a/fs/smb/client/reparse.h b/fs/smb/client/reparse.h
index 5be54878265e..2a91f64de557 100644
--- a/fs/smb/client/reparse.h
+++ b/fs/smb/client/reparse.h
@@ -18,7 +18,7 @@
  */
 #define IO_REPARSE_TAG_INTERNAL ((__u32)~0U)
 
-static inline dev_t reparse_nfs_mkdev(struct reparse_posix_data *buf)
+static inline dev_t reparse_nfs_mkdev(struct reparse_nfs_data *buf)
 {
 	u32 major, minor;
 
diff --git a/fs/smb/common/smb2pdu.h b/fs/smb/common/smb2pdu.h
index c769f9dbc0b4..0e77a4c0145a 100644
--- a/fs/smb/common/smb2pdu.h
+++ b/fs/smb/common/smb2pdu.h
@@ -1550,7 +1550,7 @@ struct reparse_symlink_data_buffer {
 	__u8	PathBuffer[]; /* Variable Length */
 } __packed;
 
-/* See MS-FSCC 2.1.2.6 and cifspdu.h for struct reparse_posix_data */
+/* See MS-FSCC 2.1.2.6 and cifspdu.h for struct reparse_nfs_data */
 
 struct validate_negotiate_info_req {
 	__le32 Capabilities;
-- 
2.20.1

Re: [PATCH 8/8] cifs: Rename posix to nfs in parse_reparse_posix() and reparse_posix_data

[Steve French]

since they are being used by default for servers supporting special
files in the "SMB3.1.1 POSIX Extensions" ... it might be appropriate
to keep a less confusing name ("NFS" for "SMB3.1.1 POSIX" could be
confusing)

On Sat, Sep 28, 2024 at 5:01 PM Pali Rohár <pali@kernel.org> wrote:
>
> This function parses NFS-style reparse points, which are used only by
> Windows NFS server since Windows Server 2012 version. This style of special
> files is not understood by Microsoft POSIX / Interix / SFU / SUA subsystems.
>
> So make it clear that parse_reparse_posix() function and reparse_posix_data
> structure are not POSIX general, but rather NFS specific.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  fs/smb/client/cifsglob.h |  2 +-
>  fs/smb/client/cifspdu.h  |  2 +-
>  fs/smb/client/reparse.c  | 14 +++++++-------
>  fs/smb/client/reparse.h  |  2 +-
>  fs/smb/common/smb2pdu.h  |  2 +-
>  5 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> index 9eae8649f90c..119537e98f77 100644
> --- a/fs/smb/client/cifsglob.h
> +++ b/fs/smb/client/cifsglob.h
> @@ -223,7 +223,7 @@ struct cifs_open_info_data {
>                 __u32 tag;
>                 union {
>                         struct reparse_data_buffer *buf;
> -                       struct reparse_posix_data *posix;
> +                       struct reparse_nfs_data *nfs;
>                 };
>         } reparse;
>         struct {
> diff --git a/fs/smb/client/cifspdu.h b/fs/smb/client/cifspdu.h
> index c3b6263060b0..fefd7e5eb170 100644
> --- a/fs/smb/client/cifspdu.h
> +++ b/fs/smb/client/cifspdu.h
> @@ -1506,7 +1506,7 @@ struct reparse_symlink_data {
>  #define NFS_SPECFILE_BLK       0x00000000004B4C42
>  #define NFS_SPECFILE_FIFO      0x000000004F464946
>  #define NFS_SPECFILE_SOCK      0x000000004B434F53
> -struct reparse_posix_data {
> +struct reparse_nfs_data {
>         __le32  ReparseTag;
>         __le16  ReparseDataLength;
>         __u16   Reserved;
> diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
> index 35e8f2e18530..a23ea2f78c09 100644
> --- a/fs/smb/client/reparse.c
> +++ b/fs/smb/client/reparse.c
> @@ -81,7 +81,7 @@ int smb2_create_reparse_symlink(const unsigned int xid, struct inode *inode,
>         return rc;
>  }
>
> -static int nfs_set_reparse_buf(struct reparse_posix_data *buf,
> +static int nfs_set_reparse_buf(struct reparse_nfs_data *buf,
>                                mode_t mode, dev_t dev,
>                                struct kvec *iov)
>  {
> @@ -120,20 +120,20 @@ static int mknod_nfs(unsigned int xid, struct inode *inode,
>                      const char *full_path, umode_t mode, dev_t dev)
>  {
>         struct cifs_open_info_data data;
> -       struct reparse_posix_data *p;
> +       struct reparse_nfs_data *p;
>         struct inode *new;
>         struct kvec iov;
>         __u8 buf[sizeof(*p) + sizeof(__le64)];
>         int rc;
>
> -       p = (struct reparse_posix_data *)buf;
> +       p = (struct reparse_nfs_data *)buf;
>         rc = nfs_set_reparse_buf(p, mode, dev, &iov);
>         if (rc)
>                 return rc;
>
>         data = (struct cifs_open_info_data) {
>                 .reparse_point = true,
> -               .reparse = { .tag = IO_REPARSE_TAG_NFS, .posix = p, },
> +               .reparse = { .tag = IO_REPARSE_TAG_NFS, .nfs = p, },
>         };
>
>         new = smb2_get_reparse_inode(&data, inode->i_sb, xid,
> @@ -313,7 +313,7 @@ int smb2_mknod_reparse(unsigned int xid, struct inode *inode,
>  }
>
>  /* See MS-FSCC 2.1.2.6 for the 'NFS' style reparse tags */
> -static int parse_reparse_posix(struct reparse_posix_data *buf,
> +static int parse_reparse_nfs(struct reparse_nfs_data *buf,
>                                struct cifs_sb_info *cifs_sb,
>                                struct cifs_open_info_data *data)
>  {
> @@ -414,7 +414,7 @@ int parse_reparse_point(struct reparse_data_buffer *buf,
>         /* See MS-FSCC 2.1.2 */
>         switch (le32_to_cpu(buf->ReparseTag)) {
>         case IO_REPARSE_TAG_NFS:
> -               return parse_reparse_posix((struct reparse_posix_data *)buf,
> +               return parse_reparse_nfs((struct reparse_nfs_data *)buf,
>                                            cifs_sb, data);
>         case IO_REPARSE_TAG_SYMLINK:
>                 return parse_reparse_symlink(
> @@ -507,7 +507,7 @@ bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
>                                  struct cifs_fattr *fattr,
>                                  struct cifs_open_info_data *data)
>  {
> -       struct reparse_posix_data *buf = data->reparse.posix;
> +       struct reparse_nfs_data *buf = data->reparse.nfs;
>         u32 tag = data->reparse.tag;
>
>         if (tag == IO_REPARSE_TAG_NFS && buf) {
> diff --git a/fs/smb/client/reparse.h b/fs/smb/client/reparse.h
> index 5be54878265e..2a91f64de557 100644
> --- a/fs/smb/client/reparse.h
> +++ b/fs/smb/client/reparse.h
> @@ -18,7 +18,7 @@
>   */
>  #define IO_REPARSE_TAG_INTERNAL ((__u32)~0U)
>
> -static inline dev_t reparse_nfs_mkdev(struct reparse_posix_data *buf)
> +static inline dev_t reparse_nfs_mkdev(struct reparse_nfs_data *buf)
>  {
>         u32 major, minor;
>
> diff --git a/fs/smb/common/smb2pdu.h b/fs/smb/common/smb2pdu.h
> index c769f9dbc0b4..0e77a4c0145a 100644
> --- a/fs/smb/common/smb2pdu.h
> +++ b/fs/smb/common/smb2pdu.h
> @@ -1550,7 +1550,7 @@ struct reparse_symlink_data_buffer {
>         __u8    PathBuffer[]; /* Variable Length */
>  } __packed;
>
> -/* See MS-FSCC 2.1.2.6 and cifspdu.h for struct reparse_posix_data */
> +/* See MS-FSCC 2.1.2.6 and cifspdu.h for struct reparse_nfs_data */
>
>  struct validate_negotiate_info_req {
>         __le32 Capabilities;
> --
> 2.20.1
>
>


-- 
Thanks,

Steve

Re: [PATCH 8/8] cifs: Rename posix to nfs in parse_reparse_posix() and reparse_posix_data

[Ralph Boehme]

[-- Attachment #1.1: Type: text/plain, Size: 7125 bytes --]

Keep in mind that the inode type info will also be available via the 
posix infolevel in the mode bits. The changes for Samba by Volker have 
already been merged last week.

Spec draft:

https://gitlab.com/samba-team/smb3-posix-spec/-/merge_requests/2

<https://www.samba.org/~slow/SMB3_POSIX/fscc_posix_extensions.html#posix-mode>

Fwiw, in a future version of POSIX-FSA we will probably say that in 
POSIX mode (per handle) the POSIX client MUST use the POSIXMode field 
for the inode type and one MUST NOT use the NFS reparse point tag for 
this. The server in POSIX mode MAY also set the reparse tag, but that's 
still open to debate from my pov.

The NFS reparse tag will be there basically for the Windows clients in 
non-POSIX mode and support will be mandatory in the server. The only 
reparse tag we'd in POSIX mode for inode type information would be 
IO_REPARSE_TAG_SYMLINK for symlinks.

-slow


On 9/29/24 6:57 AM, Steve French wrote:
> since they are being used by default for servers supporting special
> files in the "SMB3.1.1 POSIX Extensions" ... it might be appropriate
> to keep a less confusing name ("NFS" for "SMB3.1.1 POSIX" could be
> confusing)
> 
> On Sat, Sep 28, 2024 at 5:01 PM Pali Rohár <pali@kernel.org> wrote:
>>
>> This function parses NFS-style reparse points, which are used only by
>> Windows NFS server since Windows Server 2012 version. This style of special
>> files is not understood by Microsoft POSIX / Interix / SFU / SUA subsystems.
>>
>> So make it clear that parse_reparse_posix() function and reparse_posix_data
>> structure are not POSIX general, but rather NFS specific.
>>
>> Signed-off-by: Pali Rohár <pali@kernel.org>
>> ---
>>   fs/smb/client/cifsglob.h |  2 +-
>>   fs/smb/client/cifspdu.h  |  2 +-
>>   fs/smb/client/reparse.c  | 14 +++++++-------
>>   fs/smb/client/reparse.h  |  2 +-
>>   fs/smb/common/smb2pdu.h  |  2 +-
>>   5 files changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
>> index 9eae8649f90c..119537e98f77 100644
>> --- a/fs/smb/client/cifsglob.h
>> +++ b/fs/smb/client/cifsglob.h
>> @@ -223,7 +223,7 @@ struct cifs_open_info_data {
>>                  __u32 tag;
>>                  union {
>>                          struct reparse_data_buffer *buf;
>> -                       struct reparse_posix_data *posix;
>> +                       struct reparse_nfs_data *nfs;
>>                  };
>>          } reparse;
>>          struct {
>> diff --git a/fs/smb/client/cifspdu.h b/fs/smb/client/cifspdu.h
>> index c3b6263060b0..fefd7e5eb170 100644
>> --- a/fs/smb/client/cifspdu.h
>> +++ b/fs/smb/client/cifspdu.h
>> @@ -1506,7 +1506,7 @@ struct reparse_symlink_data {
>>   #define NFS_SPECFILE_BLK       0x00000000004B4C42
>>   #define NFS_SPECFILE_FIFO      0x000000004F464946
>>   #define NFS_SPECFILE_SOCK      0x000000004B434F53
>> -struct reparse_posix_data {
>> +struct reparse_nfs_data {
>>          __le32  ReparseTag;
>>          __le16  ReparseDataLength;
>>          __u16   Reserved;
>> diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
>> index 35e8f2e18530..a23ea2f78c09 100644
>> --- a/fs/smb/client/reparse.c
>> +++ b/fs/smb/client/reparse.c
>> @@ -81,7 +81,7 @@ int smb2_create_reparse_symlink(const unsigned int xid, struct inode *inode,
>>          return rc;
>>   }
>>
>> -static int nfs_set_reparse_buf(struct reparse_posix_data *buf,
>> +static int nfs_set_reparse_buf(struct reparse_nfs_data *buf,
>>                                 mode_t mode, dev_t dev,
>>                                 struct kvec *iov)
>>   {
>> @@ -120,20 +120,20 @@ static int mknod_nfs(unsigned int xid, struct inode *inode,
>>                       const char *full_path, umode_t mode, dev_t dev)
>>   {
>>          struct cifs_open_info_data data;
>> -       struct reparse_posix_data *p;
>> +       struct reparse_nfs_data *p;
>>          struct inode *new;
>>          struct kvec iov;
>>          __u8 buf[sizeof(*p) + sizeof(__le64)];
>>          int rc;
>>
>> -       p = (struct reparse_posix_data *)buf;
>> +       p = (struct reparse_nfs_data *)buf;
>>          rc = nfs_set_reparse_buf(p, mode, dev, &iov);
>>          if (rc)
>>                  return rc;
>>
>>          data = (struct cifs_open_info_data) {
>>                  .reparse_point = true,
>> -               .reparse = { .tag = IO_REPARSE_TAG_NFS, .posix = p, },
>> +               .reparse = { .tag = IO_REPARSE_TAG_NFS, .nfs = p, },
>>          };
>>
>>          new = smb2_get_reparse_inode(&data, inode->i_sb, xid,
>> @@ -313,7 +313,7 @@ int smb2_mknod_reparse(unsigned int xid, struct inode *inode,
>>   }
>>
>>   /* See MS-FSCC 2.1.2.6 for the 'NFS' style reparse tags */
>> -static int parse_reparse_posix(struct reparse_posix_data *buf,
>> +static int parse_reparse_nfs(struct reparse_nfs_data *buf,
>>                                 struct cifs_sb_info *cifs_sb,
>>                                 struct cifs_open_info_data *data)
>>   {
>> @@ -414,7 +414,7 @@ int parse_reparse_point(struct reparse_data_buffer *buf,
>>          /* See MS-FSCC 2.1.2 */
>>          switch (le32_to_cpu(buf->ReparseTag)) {
>>          case IO_REPARSE_TAG_NFS:
>> -               return parse_reparse_posix((struct reparse_posix_data *)buf,
>> +               return parse_reparse_nfs((struct reparse_nfs_data *)buf,
>>                                             cifs_sb, data);
>>          case IO_REPARSE_TAG_SYMLINK:
>>                  return parse_reparse_symlink(
>> @@ -507,7 +507,7 @@ bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
>>                                   struct cifs_fattr *fattr,
>>                                   struct cifs_open_info_data *data)
>>   {
>> -       struct reparse_posix_data *buf = data->reparse.posix;
>> +       struct reparse_nfs_data *buf = data->reparse.nfs;
>>          u32 tag = data->reparse.tag;
>>
>>          if (tag == IO_REPARSE_TAG_NFS && buf) {
>> diff --git a/fs/smb/client/reparse.h b/fs/smb/client/reparse.h
>> index 5be54878265e..2a91f64de557 100644
>> --- a/fs/smb/client/reparse.h
>> +++ b/fs/smb/client/reparse.h
>> @@ -18,7 +18,7 @@
>>    */
>>   #define IO_REPARSE_TAG_INTERNAL ((__u32)~0U)
>>
>> -static inline dev_t reparse_nfs_mkdev(struct reparse_posix_data *buf)
>> +static inline dev_t reparse_nfs_mkdev(struct reparse_nfs_data *buf)
>>   {
>>          u32 major, minor;
>>
>> diff --git a/fs/smb/common/smb2pdu.h b/fs/smb/common/smb2pdu.h
>> index c769f9dbc0b4..0e77a4c0145a 100644
>> --- a/fs/smb/common/smb2pdu.h
>> +++ b/fs/smb/common/smb2pdu.h
>> @@ -1550,7 +1550,7 @@ struct reparse_symlink_data_buffer {
>>          __u8    PathBuffer[]; /* Variable Length */
>>   } __packed;
>>
>> -/* See MS-FSCC 2.1.2.6 and cifspdu.h for struct reparse_posix_data */
>> +/* See MS-FSCC 2.1.2.6 and cifspdu.h for struct reparse_nfs_data */
>>
>>   struct validate_negotiate_info_req {
>>          __le32 Capabilities;
>> --
>> 2.20.1
>>
>>
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 8/8] cifs: Rename posix to nfs in parse_reparse_posix() and reparse_posix_data

[Pali Rohár]

Hello Ralph, thank you for information. So in case Samba is not going to
use IO_REPARSE_TAG_NFS as primary way to serve special files, then it
still makes sense to do this structure rename with my patch?



Anyway, Windows clients mostly do not use IO_REPARSE_TAG_NFS. From my
knowledge on Windows this tag is used only by Windows NFS server.
So scenario when Windows sends IO_REPARSE_TAG_NFS over SMB would be
rare... It would be needed to export NFS share via Windows NFS server
from SMB mount connected to Samba server.

Note that Windows NFS client stores data about special files in EAs. So
for example if you mount export from Linux NFS server by Windows NFS
client, then NFS symlink would be visible to Windows applications as
regular file with "NfsSymlinkTargetName" EA. More info is in this email:
https://marc.info/?l=samba-technical&m=121423255119680

And this is what are Windows applications using if they want to access
data of special files. So application access "NfsSymlinkTargetName" EA
and not IO_REPARSE_TAG_NFS reparse tag.

To my knowledge neither Samba, nor Linux CIFS client supports
"NfsSymlinkTargetName" EA for creating or parsing symlink.

On Sunday 29 September 2024 11:09:44 Ralph Boehme wrote:
> Keep in mind that the inode type info will also be available via the posix
> infolevel in the mode bits. The changes for Samba by Volker have already
> been merged last week.
> 
> Spec draft:
> 
> https://gitlab.com/samba-team/smb3-posix-spec/-/merge_requests/2
> 
> <https://www.samba.org/~slow/SMB3_POSIX/fscc_posix_extensions.html#posix-mode>
> 
> Fwiw, in a future version of POSIX-FSA we will probably say that in POSIX
> mode (per handle) the POSIX client MUST use the POSIXMode field for the
> inode type and one MUST NOT use the NFS reparse point tag for this. The
> server in POSIX mode MAY also set the reparse tag, but that's still open to
> debate from my pov.
> 
> The NFS reparse tag will be there basically for the Windows clients in
> non-POSIX mode and support will be mandatory in the server. The only reparse
> tag we'd in POSIX mode for inode type information would be
> IO_REPARSE_TAG_SYMLINK for symlinks.
> 
> -slow
> 
> 
> On 9/29/24 6:57 AM, Steve French wrote:
> > since they are being used by default for servers supporting special
> > files in the "SMB3.1.1 POSIX Extensions" ... it might be appropriate
> > to keep a less confusing name ("NFS" for "SMB3.1.1 POSIX" could be
> > confusing)
> > 
> > On Sat, Sep 28, 2024 at 5:01 PM Pali Rohár <pali@kernel.org> wrote:
> > > 
> > > This function parses NFS-style reparse points, which are used only by
> > > Windows NFS server since Windows Server 2012 version. This style of special
> > > files is not understood by Microsoft POSIX / Interix / SFU / SUA subsystems.
> > > 
> > > So make it clear that parse_reparse_posix() function and reparse_posix_data
> > > structure are not POSIX general, but rather NFS specific.
> > > 
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > ---
> > >   fs/smb/client/cifsglob.h |  2 +-
> > >   fs/smb/client/cifspdu.h  |  2 +-
> > >   fs/smb/client/reparse.c  | 14 +++++++-------
> > >   fs/smb/client/reparse.h  |  2 +-
> > >   fs/smb/common/smb2pdu.h  |  2 +-
> > >   5 files changed, 11 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> > > index 9eae8649f90c..119537e98f77 100644
> > > --- a/fs/smb/client/cifsglob.h
> > > +++ b/fs/smb/client/cifsglob.h
> > > @@ -223,7 +223,7 @@ struct cifs_open_info_data {
> > >                  __u32 tag;
> > >                  union {
> > >                          struct reparse_data_buffer *buf;
> > > -                       struct reparse_posix_data *posix;
> > > +                       struct reparse_nfs_data *nfs;
> > >                  };
> > >          } reparse;
> > >          struct {
> > > diff --git a/fs/smb/client/cifspdu.h b/fs/smb/client/cifspdu.h
> > > index c3b6263060b0..fefd7e5eb170 100644
> > > --- a/fs/smb/client/cifspdu.h
> > > +++ b/fs/smb/client/cifspdu.h
> > > @@ -1506,7 +1506,7 @@ struct reparse_symlink_data {
> > >   #define NFS_SPECFILE_BLK       0x00000000004B4C42
> > >   #define NFS_SPECFILE_FIFO      0x000000004F464946
> > >   #define NFS_SPECFILE_SOCK      0x000000004B434F53
> > > -struct reparse_posix_data {
> > > +struct reparse_nfs_data {
> > >          __le32  ReparseTag;
> > >          __le16  ReparseDataLength;
> > >          __u16   Reserved;
> > > diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
> > > index 35e8f2e18530..a23ea2f78c09 100644
> > > --- a/fs/smb/client/reparse.c
> > > +++ b/fs/smb/client/reparse.c
> > > @@ -81,7 +81,7 @@ int smb2_create_reparse_symlink(const unsigned int xid, struct inode *inode,
> > >          return rc;
> > >   }
> > > 
> > > -static int nfs_set_reparse_buf(struct reparse_posix_data *buf,
> > > +static int nfs_set_reparse_buf(struct reparse_nfs_data *buf,
> > >                                 mode_t mode, dev_t dev,
> > >                                 struct kvec *iov)
> > >   {
> > > @@ -120,20 +120,20 @@ static int mknod_nfs(unsigned int xid, struct inode *inode,
> > >                       const char *full_path, umode_t mode, dev_t dev)
> > >   {
> > >          struct cifs_open_info_data data;
> > > -       struct reparse_posix_data *p;
> > > +       struct reparse_nfs_data *p;
> > >          struct inode *new;
> > >          struct kvec iov;
> > >          __u8 buf[sizeof(*p) + sizeof(__le64)];
> > >          int rc;
> > > 
> > > -       p = (struct reparse_posix_data *)buf;
> > > +       p = (struct reparse_nfs_data *)buf;
> > >          rc = nfs_set_reparse_buf(p, mode, dev, &iov);
> > >          if (rc)
> > >                  return rc;
> > > 
> > >          data = (struct cifs_open_info_data) {
> > >                  .reparse_point = true,
> > > -               .reparse = { .tag = IO_REPARSE_TAG_NFS, .posix = p, },
> > > +               .reparse = { .tag = IO_REPARSE_TAG_NFS, .nfs = p, },
> > >          };
> > > 
> > >          new = smb2_get_reparse_inode(&data, inode->i_sb, xid,
> > > @@ -313,7 +313,7 @@ int smb2_mknod_reparse(unsigned int xid, struct inode *inode,
> > >   }
> > > 
> > >   /* See MS-FSCC 2.1.2.6 for the 'NFS' style reparse tags */
> > > -static int parse_reparse_posix(struct reparse_posix_data *buf,
> > > +static int parse_reparse_nfs(struct reparse_nfs_data *buf,
> > >                                 struct cifs_sb_info *cifs_sb,
> > >                                 struct cifs_open_info_data *data)
> > >   {
> > > @@ -414,7 +414,7 @@ int parse_reparse_point(struct reparse_data_buffer *buf,
> > >          /* See MS-FSCC 2.1.2 */
> > >          switch (le32_to_cpu(buf->ReparseTag)) {
> > >          case IO_REPARSE_TAG_NFS:
> > > -               return parse_reparse_posix((struct reparse_posix_data *)buf,
> > > +               return parse_reparse_nfs((struct reparse_nfs_data *)buf,
> > >                                             cifs_sb, data);
> > >          case IO_REPARSE_TAG_SYMLINK:
> > >                  return parse_reparse_symlink(
> > > @@ -507,7 +507,7 @@ bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
> > >                                   struct cifs_fattr *fattr,
> > >                                   struct cifs_open_info_data *data)
> > >   {
> > > -       struct reparse_posix_data *buf = data->reparse.posix;
> > > +       struct reparse_nfs_data *buf = data->reparse.nfs;
> > >          u32 tag = data->reparse.tag;
> > > 
> > >          if (tag == IO_REPARSE_TAG_NFS && buf) {
> > > diff --git a/fs/smb/client/reparse.h b/fs/smb/client/reparse.h
> > > index 5be54878265e..2a91f64de557 100644
> > > --- a/fs/smb/client/reparse.h
> > > +++ b/fs/smb/client/reparse.h
> > > @@ -18,7 +18,7 @@
> > >    */
> > >   #define IO_REPARSE_TAG_INTERNAL ((__u32)~0U)
> > > 
> > > -static inline dev_t reparse_nfs_mkdev(struct reparse_posix_data *buf)
> > > +static inline dev_t reparse_nfs_mkdev(struct reparse_nfs_data *buf)
> > >   {
> > >          u32 major, minor;
> > > 
> > > diff --git a/fs/smb/common/smb2pdu.h b/fs/smb/common/smb2pdu.h
> > > index c769f9dbc0b4..0e77a4c0145a 100644
> > > --- a/fs/smb/common/smb2pdu.h
> > > +++ b/fs/smb/common/smb2pdu.h
> > > @@ -1550,7 +1550,7 @@ struct reparse_symlink_data_buffer {
> > >          __u8    PathBuffer[]; /* Variable Length */
> > >   } __packed;
> > > 
> > > -/* See MS-FSCC 2.1.2.6 and cifspdu.h for struct reparse_posix_data */
> > > +/* See MS-FSCC 2.1.2.6 and cifspdu.h for struct reparse_nfs_data */
> > > 
> > >   struct validate_negotiate_info_req {
> > >          __le32 Capabilities;
> > > --
> > > 2.20.1
> > > 
> > > 
> > 
> > 
> 

Re: [PATCH 8/8] cifs: Rename posix to nfs in parse_reparse_posix() and reparse_posix_data

[Ralph Boehme]

[-- Attachment #1.1: Type: text/plain, Size: 1662 bytes --]

On 9/29/24 11:26 AM, Pali Rohár wrote:
> Hello Ralph, thank you for information. So in case Samba is not
> going to use IO_REPARSE_TAG_NFS as primary way to serve special
> files, then it still makes sense to do this structure rename with my
> patch?

that's up to Paulo and Steve. I can only talk protocol/spec and server. :)

> Anyway, Windows clients mostly do not use IO_REPARSE_TAG_NFS.

They don't *create* it, but they can *read* and present it. But I guess 
that's what you meant.

> From my knowledge on Windows this tag is used only by Windows NFS
> server. So scenario when Windows sends IO_REPARSE_TAG_NFS over SMB
> would be rare... It would be needed to export NFS share via Windows
> NFS server from SMB mount connected to Samba server.

That's out of scope as far as SMB3 POSIX Extensions and I are concerned. :)

> Note that Windows NFS client stores data about special files in EAs.
> So for example if you mount export from Linux NFS server by Windows
> NFS client, then NFS symlink would be visible to Windows
> applications as regular file with "NfsSymlinkTargetName" EA. More
> info is in this email: https://marc.info/?l=samba-
> technical&m=121423255119680
> 
> And this is what are Windows applications using if they want to
> access data of special files. So application access
> "NfsSymlinkTargetName" EA and not IO_REPARSE_TAG_NFS reparse tag.

For symlinks SMB3 POSIX Extensions will use what Windows uses natively: 
IO_REPARSE_TAG_SYMLINK.

> To my knowledge neither Samba, nor Linux CIFS client supports 
> "NfsSymlinkTargetName" EA for creating or parsing symlink.

for Samba: yup.

-slow

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 8/8] cifs: Rename posix to nfs in parse_reparse_posix() and reparse_posix_data

[Steve French]

On Sun, Sep 29, 2024 at 7:52 AM Ralph Boehme <slow@samba.org> wrote:
>
> On 9/29/24 11:26 AM, Pali Rohár wrote:
> > Hello Ralph, thank you for information. So in case Samba is not
> > going to use IO_REPARSE_TAG_NFS as primary way to serve special
> > files, then it still makes sense to do this structure rename with my
> > patch?
>
> that's up to Paulo and Steve. I can only talk protocol/spec and server. :)

I don't think renaming that struct helps since to various servers we
will have to use reparse points to save special files, and could be
confusing to keep mentioning "nfs" for use cases in the client code
where not really related much to NFS.

> > Anyway, Windows clients mostly do not use IO_REPARSE_TAG_NFS.
>
> They don't *create* it, but they can *read* and present it. But I guess
> that's what you meant.
>
> > From my knowledge on Windows this tag is used only by Windows NFS
> > server. So scenario when Windows sends IO_REPARSE_TAG_NFS over SMB
> > would be rare... It would be needed to export NFS share via Windows
> > NFS server from SMB mount connected to Samba server.
>
> That's out of scope as far as SMB3 POSIX Extensions and I are concerned. :)
>
> > Note that Windows NFS client stores data about special files in EAs.
> > So for example if you mount export from Linux NFS server by Windows
> > NFS client, then NFS symlink would be visible to Windows
> > applications as regular file with "NfsSymlinkTargetName" EA. More
> > info is in this email: https://marc.info/?l=samba-
> > technical&m=121423255119680
> >
> > And this is what are Windows applications using if they want to
> > access data of special files. So application access
> > "NfsSymlinkTargetName" EA and not IO_REPARSE_TAG_NFS reparse tag.
>
> For symlinks SMB3 POSIX Extensions will use what Windows uses natively:
> IO_REPARSE_TAG_SYMLINK.
>
> > To my knowledge neither Samba, nor Linux CIFS client supports
> > "NfsSymlinkTargetName" EA for creating or parsing symlink.
>
> for Samba: yup.

Yes.  We would only want to support reading the NfsSymlinkTargetName
for the rare
cases where it would come up, but the default for creating symlinks on
the client
would be to use "mfysmlinks" (safer, client only symlinks, like Apple) when the
mfsymlinks mount parm is specified, otherwise use the default Windows
symlink type
when creating a new (server side) symlink remotely.

-- 
Thanks,

Steve