Special files broken against Samba master

Special files broken against Samba master

[Ralph Boehme]

[-- Attachment #1.1.1: Type: text/plain, Size: 2539 bytes --]

Hi!

Against Samba master special files are broken with posix extensions.

On the Samba server:

root@smb3-posix:/home/samba# ls -li /srv/samba/test/posix/
insgesamt 20
131958 brw-r--r--  1 root  root  0, 0 15. Nov 12:04 blockdev
131965 crw-r--r--  1 root  root  1, 1 15. Nov 12:04 chardev
131966 prw-r--r--  1 samba samba    0 15. Nov 12:05 fif
131953 -rw-rwxrw-+ 2 samba samba    4 18. Nov 11:37 file
131953 -rw-rwxrw-+ 2 samba samba    4 18. Nov 11:37 hardlink
131957 lrwxrwxrwx  1 samba samba    4 15. Nov 12:03 symlink -> file
131954 -rwxrwxr-x+ 1 samba samba    0 18. Nov 15:28 symlinkoversmb

# mount.cifs //127.0.0.1/posix /mnt/smb3unix/ -o 
username=samba,password=x,posix

# ls -li /mnt/smb3unix/posix/
ls: cannot access '/mnt/smb3unix/posix/blockdev': No data available
ls: cannot access '/mnt/smb3unix/posix/chardev': No data available
ls: cannot access '/mnt/smb3unix/posix/symlinkoversmb': No data available
ls: cannot access '/mnt/smb3unix/posix/fifo': No data available
ls: cannot access '/mnt/smb3unix/posix/symlink': No data available
total 16
      ? -????????? ? ?    ?     ?            ? blockdev
      ? -????????? ? ?    ?     ?            ? chardev
      ? -????????? ? ?    ?     ?            ? fifo
131953 -rw-rwxrw- 2 root samba 4 Nov 18 11:37 file
131953 -rw-rwxrw- 2 root samba 4 Nov 18 11:37 hardlink
      ? -????????? ? ?    ?     ?            ? symlink
      ? -????????? ? ?    ?     ?            ? symlinkoversmb

I have two WIP patches that fix this as a side effect of adding support 
to using the new POSIX type as discussed at SDC, patches attached:

root@smb3-posix:/home/samba# ls -li /mnt/smb3unix/posix/
insgesamt 21
131958 brw-r--r-- 1 root root  0, 0 15. Nov 12:04 blockdev
131965 crw-r--r-- 1 root root  1, 1 15. Nov 12:04 chardev
131966 prw-r--r-- 1 root samba    0 15. Nov 12:05 fifo
131953 -rw-rwxrw- 2 root samba    4 18. Nov 11:37 file
131953 -rw-rwxrw- 2 root samba    4 18. Nov 11:37 hardlink
131957 lrwxrwxrwx 1 root samba    4 15. Nov 12:03 symlink -> file
131954 lrwxrwxr-x 1 root samba   23 18. Nov 15:28 symlinkoversmb -> 
mnt/smb3unix/posix/file

I'm also optimizing away unneeded reparse point queries 
(create+fsctl+close) for nfs reparse points where we only need the file 
type which we now get and use via the smb3 posix type.

I'm sure I'm doing things wrong. Can you help me getting this across the 
line?

-slow

[1] 
<https://www.samba.org/~slow/SMB3_POSIX/fscc_posix_extensions.html#posix-file-type-definition>

[-- Attachment #1.1.2: posix-type.patch --]
[-- Type: text/x-patch, Size: 13207 bytes --]

From a57c32da874f285af266b7fbbaefb7940991d049 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Fri, 15 Nov 2024 13:15:50 +0100
Subject: [PATCH 1/3] fs/smb/client: avoid querying SMB2_OP_QUERY_WSL_EA for
 SMB3 POSIX

---
 fs/smb/client/smb2inode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index e49d0c25eb03..ab069d5285c5 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -941,7 +941,8 @@ int smb2_query_path_info(const unsigned int xid,
 		if (rc || !data->reparse_point)
 			goto out;
 
-		cmds[num_cmds++] = SMB2_OP_QUERY_WSL_EA;
+		if (!tcon->posix_extensions)
+			cmds[num_cmds++] = SMB2_OP_QUERY_WSL_EA;
 		/*
 		 * Skip SMB2_OP_GET_REPARSE if symlink already parsed in create
 		 * response.
-- 
2.47.0


From afd2dce84b1ae970b2ec4421948194746f542cf6 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Fri, 15 Nov 2024 19:21:04 +0100
Subject: [PATCH 2/3] fs/smb/client: Implement new SMB3 POSIX type

Fixes special files against current Samba.

On the Samba server:

insgesamt 20
131958 brw-r--r--  1 root  root  0, 0 15. Nov 12:04 blockdev
131965 crw-r--r--  1 root  root  1, 1 15. Nov 12:04 chardev
131966 prw-r--r--  1 samba samba    0 15. Nov 12:05 fifo
131953 -rw-rwxrw-+ 2 samba samba    4 18. Nov 11:37 file
131953 -rw-rwxrw-+ 2 samba samba    4 18. Nov 11:37 hardlink
131957 lrwxrwxrwx  1 samba samba    4 15. Nov 12:03 symlink -> file
131954 -rwxrwxr-x+ 1 samba samba    0 18. Nov 15:28 symlinkoversmb

Before:

ls: cannot access '/mnt/smb3unix/posix/blockdev': No data available
ls: cannot access '/mnt/smb3unix/posix/chardev': No data available
ls: cannot access '/mnt/smb3unix/posix/symlinkoversmb': No data available
ls: cannot access '/mnt/smb3unix/posix/fifo': No data available
ls: cannot access '/mnt/smb3unix/posix/symlink': No data available
total 16
     ? -????????? ? ?    ?     ?            ? blockdev
     ? -????????? ? ?    ?     ?            ? chardev
     ? -????????? ? ?    ?     ?            ? fifo
131953 -rw-rwxrw- 2 root samba 4 Nov 18 11:37 file
131953 -rw-rwxrw- 2 root samba 4 Nov 18 11:37 hardlink
     ? -????????? ? ?    ?     ?            ? symlink
     ? -????????? ? ?    ?     ?            ? symlinkoversmb

After:

insgesamt 21
131958 brw-r--r-- 1 root root  0, 0 15. Nov 12:04 blockdev
131965 crw-r--r-- 1 root root  1, 1 15. Nov 12:04 chardev
131966 prw-r--r-- 1 root samba    0 15. Nov 12:05 fifo
131953 -rw-rwxrw- 2 root samba    4 18. Nov 11:37 file
131953 -rw-rwxrw- 2 root samba    4 18. Nov 11:37 hardlink
131957 lrwxrwxrwx 1 root samba    4 15. Nov 12:03 symlink -> file
131954 lrwxrwxr-x 1 root samba   23 18. Nov 15:28 symlinkoversmb -> mnt/smb3unix/posix/file
---
 fs/smb/client/cifsproto.h |  1 +
 fs/smb/client/inode.c     | 89 +++++++++++++++++++++++++++++++++++----
 fs/smb/client/readdir.c   | 35 +++++++--------
 fs/smb/client/reparse.c   | 84 ++++++++++++++++++++++--------------
 4 files changed, 149 insertions(+), 60 deletions(-)

diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index d312ea9776ce..20ee7feb9c49 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -676,6 +676,7 @@ int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 int cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 		       struct dentry *dentry, struct cifs_tcon *tcon,
 		       const char *full_path, umode_t mode, dev_t dev);
+umode_t wire_mode_to_posix(u32 wire);
 
 #ifdef CONFIG_CIFS_DFS_UPCALL
 static inline int get_dfs_path(const unsigned int xid, struct cifs_ses *ses,
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 72ebd72dd02b..0dcf9e3635ff 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -724,6 +724,84 @@ static int cifs_sfu_mode(struct cifs_fattr *fattr, const unsigned char *path,
 #endif
 }
 
+#define POSIX_TYPE_FILE    0
+#define POSIX_TYPE_DIR     1
+#define POSIX_TYPE_SYMLINK 2
+#define POSIX_TYPE_CHARDEV 3
+#define POSIX_TYPE_BLKDEV  4
+#define POSIX_TYPE_FIFO    5
+#define POSIX_TYPE_SOCKET  6
+
+#define POSIX_X_OTH      0000001
+#define POSIX_W_OTH      0000002
+#define POSIX_R_OTH      0000004
+#define POSIX_X_GRP      0000010
+#define POSIX_W_GRP      0000020
+#define POSIX_R_GRP      0000040
+#define POSIX_X_USR      0000100
+#define POSIX_W_USR      0000200
+#define POSIX_R_USR      0000400
+#define POSIX_STICKY     0001000
+#define POSIX_SET_GID    0002000
+#define POSIX_SET_UID    0004000
+
+#define POSIX_OTH_MASK      0000007
+#define POSIX_GRP_MASK      0000070
+#define POSIX_USR_MASK      0000700
+#define POSIX_PERM_MASK     0000777
+#define POSIX_FILETYPE_MASK 0070000
+
+#define POSIX_FILETYPE_SHIFT 12
+
+static u32 wire_perms_to_posix(u32 wire)
+{
+        u32 mode = 0;
+
+        mode |= (wire & POSIX_X_OTH) ? S_IXOTH : 0;
+        mode |= (wire & POSIX_W_OTH) ? S_IWOTH : 0;
+        mode |= (wire & POSIX_R_OTH) ? S_IROTH : 0;
+        mode |= (wire & POSIX_X_GRP) ? S_IXGRP : 0;
+        mode |= (wire & POSIX_W_GRP) ? S_IWGRP : 0;
+        mode |= (wire & POSIX_R_GRP) ? S_IRGRP : 0;
+        mode |= (wire & POSIX_X_USR) ? S_IXUSR : 0;
+        mode |= (wire & POSIX_W_USR) ? S_IWUSR : 0;
+        mode |= (wire & POSIX_R_USR) ? S_IRUSR : 0;
+        mode |= (wire & POSIX_STICKY) ? S_ISVTX : 0;
+        mode |= (wire & POSIX_SET_GID) ? S_ISGID : 0;
+        mode |= (wire & POSIX_SET_UID) ? S_ISUID : 0;
+
+        return mode;
+}
+
+static u32 posix_filetypes[] = {
+	S_IFREG,
+	S_IFDIR,
+	S_IFLNK,
+	S_IFCHR,
+	S_IFBLK,
+	S_IFIFO,
+	S_IFSOCK
+};
+
+static u32 wire_filetype_to_posix(u32 wire_type)
+{
+	if (wire_type >= ARRAY_SIZE(posix_filetypes)) {
+		pr_warn("Unexpected type %u", wire_type);
+		return 0;
+	}
+	return posix_filetypes[wire_type];
+}
+
+umode_t wire_mode_to_posix(u32 wire)
+{
+	u32 wire_type;
+	u32 mode;
+
+	wire_type = (wire & POSIX_FILETYPE_MASK) >> POSIX_FILETYPE_SHIFT;
+	mode = (wire_perms_to_posix(wire) | wire_filetype_to_posix(wire_type));
+	return (umode_t)mode;
+}
+
 /* Fill a cifs_fattr struct with info from POSIX info struct */
 static void smb311_posix_info_to_fattr(struct cifs_fattr *fattr,
 				       struct cifs_open_info_data *data,
@@ -760,20 +838,13 @@ static void smb311_posix_info_to_fattr(struct cifs_fattr *fattr,
 	fattr->cf_bytes = le64_to_cpu(info->AllocationSize);
 	fattr->cf_createtime = le64_to_cpu(info->CreationTime);
 	fattr->cf_nlink = le32_to_cpu(info->HardLinks);
-	fattr->cf_mode = (umode_t) le32_to_cpu(info->Mode);
+	fattr->cf_mode = wire_mode_to_posix(le32_to_cpu(info->Mode));
 
 	if (cifs_open_data_reparse(data) &&
 	    cifs_reparse_point_to_fattr(cifs_sb, fattr, data))
 		goto out_reparse;
 
-	fattr->cf_mode &= ~S_IFMT;
-	if (fattr->cf_cifsattrs & ATTR_DIRECTORY) {
-		fattr->cf_mode |= S_IFDIR;
-		fattr->cf_dtype = DT_DIR;
-	} else { /* file */
-		fattr->cf_mode |= S_IFREG;
-		fattr->cf_dtype = DT_REG;
-	}
+	fattr->cf_dtype = S_DT(fattr->cf_mode);
 
 out_reparse:
 	if (S_ISLNK(fattr->cf_mode)) {
diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index b3a8f9c6fcff..28071b2be88c 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -241,31 +241,28 @@ cifs_posix_to_fattr(struct cifs_fattr *fattr, struct smb2_posix_info *info,
 	fattr->cf_nlink = le32_to_cpu(info->HardLinks);
 	fattr->cf_cifsattrs = le32_to_cpu(info->DosAttributes);
 
-	/*
-	 * Since we set the inode type below we need to mask off
-	 * to avoid strange results if bits set above.
-	 * XXX: why not make server&client use the type bits?
-	 */
-	fattr->cf_mode = le32_to_cpu(info->Mode) & ~S_IFMT;
+	if (fattr->cf_cifsattrs & ATTR_REPARSE) {
+		fattr->cf_cifstag = le32_to_cpu(info->ReparseTag);
+	}
+
+	fattr->cf_mode = wire_mode_to_posix(le32_to_cpu(info->Mode));
+	fattr->cf_dtype = S_DT(le32_to_cpu(info->Mode));
+
+	switch (fattr->cf_mode & S_IFMT) {
+	case S_IFLNK:
+	case S_IFBLK:
+	case S_IFCHR:
+		fattr->cf_flags |= CIFS_FATTR_NEED_REVAL;
+		break;
+	default:
+		break;
+	}
 
 	cifs_dbg(FYI, "posix fattr: dev %d, reparse %d, mode %o\n",
 		 le32_to_cpu(info->DeviceId),
 		 le32_to_cpu(info->ReparseTag),
 		 le32_to_cpu(info->Mode));
 
-	if (fattr->cf_cifsattrs & ATTR_DIRECTORY) {
-		fattr->cf_mode |= S_IFDIR;
-		fattr->cf_dtype = DT_DIR;
-	} else {
-		/*
-		 * mark anything that is not a dir as regular
-		 * file. special files should have the REPARSE
-		 * attribute and will be marked as needing revaluation
-		 */
-		fattr->cf_mode |= S_IFREG;
-		fattr->cf_dtype = DT_REG;
-	}
-
 	sid_to_id(cifs_sb, &parsed.owner, fattr, SIDOWNER);
 	sid_to_id(cifs_sb, &parsed.group, fattr, SIDGROUP);
 }
diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index 74abbdf5026c..d392b9c53e43 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -661,44 +661,60 @@ static void wsl_to_fattr(struct cifs_open_info_data *data,
 	fattr->cf_dtype = S_DT(fattr->cf_mode);
 }
 
-bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
-				 struct cifs_fattr *fattr,
-				 struct cifs_open_info_data *data)
+static bool posix_reparse_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;
-	u32 tag = data->reparse.tag;
 
-	if (tag == IO_REPARSE_TAG_NFS && buf) {
-		if (le16_to_cpu(buf->ReparseDataLength) < sizeof(buf->InodeType))
+	if (buf == NULL) {
+		return true;
+	}
+
+	if (le16_to_cpu(buf->ReparseDataLength) < sizeof(buf->InodeType)) {
+		WARN_ON_ONCE(1);
+		return false;
+	}
+
+	switch (le64_to_cpu(buf->InodeType)) {
+	case NFS_SPECFILE_CHR:
+		if (le16_to_cpu(buf->ReparseDataLength) != sizeof(buf->InodeType) + 8) {
+			WARN_ON_ONCE(1);
 			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;
-		case NFS_SPECFILE_FIFO:
-			fattr->cf_mode |= S_IFIFO;
-			break;
-		case NFS_SPECFILE_SOCK:
-			fattr->cf_mode |= S_IFSOCK;
-			break;
-		case NFS_SPECFILE_LNK:
-			fattr->cf_mode |= S_IFLNK;
-			break;
-		default:
+		}
+		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) {
 			WARN_ON_ONCE(1);
 			return false;
 		}
-		goto out;
+		fattr->cf_mode |= S_IFBLK;
+		fattr->cf_rdev = reparse_mkdev(buf->DataBuffer);
+		break;
+	case NFS_SPECFILE_FIFO:
+		fattr->cf_mode |= S_IFIFO;
+		break;
+	case NFS_SPECFILE_SOCK:
+		fattr->cf_mode |= S_IFSOCK;
+		break;
+	case NFS_SPECFILE_LNK:
+		fattr->cf_mode |= S_IFLNK;
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		return false;
 	}
+	return true;
+}
+
+bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
+				 struct cifs_fattr *fattr,
+				 struct cifs_open_info_data *data)
+{
+	u32 tag = data->reparse.tag;
+	bool ok;
 
 	switch (tag) {
 	case IO_REPARSE_TAG_INTERNAL:
@@ -718,15 +734,19 @@ bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
 	case IO_REPARSE_TAG_LX_BLK:
 		wsl_to_fattr(data, cifs_sb, tag, fattr);
 		break;
+	case IO_REPARSE_TAG_NFS:
+		ok = posix_reparse_to_fattr(cifs_sb, fattr, data);
+		if (!ok)
+			return false;
+		break;
 	case 0: /* SMB1 symlink */
 	case IO_REPARSE_TAG_SYMLINK:
-	case IO_REPARSE_TAG_NFS:
 		fattr->cf_mode |= S_IFLNK;
 		break;
 	default:
 		return false;
 	}
-out:
+
 	fattr->cf_dtype = S_DT(fattr->cf_mode);
 	return true;
 }
-- 
2.47.0


From 436e2dd352980f0be7f43e02c8c24d558a7d54bc Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Mon, 25 Nov 2024 16:19:56 +0100
Subject: [PATCH 3/3] fs/smb/client: cifs_prime_dcache() for SMB3 POSIX reparse
 points

---
 fs/smb/client/readdir.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index 28071b2be88c..d2137bcb8d87 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -71,6 +71,8 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name,
 	struct inode *inode;
 	struct super_block *sb = parent->d_sb;
 	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
+	bool posix = cifs_sb_master_tcon(cifs_sb)->posix_extensions;
+	bool reparse_need_reval = false;
 	DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
 	int rc;
 
@@ -85,7 +87,21 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name,
 		 * this spares us an invalidation.
 		 */
 retry:
-		if ((fattr->cf_cifsattrs & ATTR_REPARSE) ||
+		if (posix) {
+			switch (fattr->cf_mode & S_IFMT) {
+			case S_IFLNK:
+			case S_IFBLK:
+			case S_IFCHR:
+				reparse_need_reval = true;
+				break;
+			default:
+				break;
+			}
+		} else if (fattr->cf_cifsattrs & ATTR_REPARSE) {
+			reparse_need_reval = true;
+		}
+
+		if (reparse_need_reval ||
 		    (fattr->cf_flags & CIFS_FATTR_NEED_REVAL))
 			return;
 
-- 
2.47.0


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

Re: Special files broken against Samba master

[Paulo Alcantara]

Ralph Boehme <slow@samba.org> writes:

> I'm also optimizing away unneeded reparse point queries 
> (create+fsctl+close) for nfs reparse points where we only need the file 
> type which we now get and use via the smb3 posix type.

Awesome!  Yeah, we don't need to revalidate the special files with
SMB3.1.1 POSIX as we can already parse file type directly from query dir
response.  We could that in separate patches, so no worries.  This will
definitely help with getdents(2).

> I'm sure I'm doing things wrong. Can you help me getting this across the 
> line?

Patches look good, thanks.  It would be great making sure it didn't
regress against NFS server on Windows, but Steve can do that.

Thanks Ralph!

Re: Special files broken against Samba master

[Ralph Boehme]

[-- Attachment #1.1.1: Type: text/plain, Size: 534 bytes --]

On 12/2/24 11:40 PM, Paulo Alcantara wrote:
> Ralph Boehme <slow@samba.org> writes:
>> I'm sure I'm doing things wrong. Can you help me getting this across the
>> line?
> 
> Patches look good, thanks.  It would be great making sure it didn't
> regress against NFS server on Windows, but Steve can do that.

great, thanks for taking a look! I was expecting I was likely doing some 
stupid beginner mistakes, but if you think the changes look good, here's 
a v2 with just my +1 added, without code changes.

Thanks!
-slow

[-- Attachment #1.1.2: posix-type-v2.patch --]
[-- Type: text/x-patch, Size: 13343 bytes --]

From d73e1672f001b532af18d92ce361f535b5c3e91d Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Fri, 15 Nov 2024 13:15:50 +0100
Subject: [PATCH 1/3] fs/smb/client: avoid querying SMB2_OP_QUERY_WSL_EA for
 SMB3 POSIX

Signed-off-by: Ralph Boehme <slow@samba.org>
---
 fs/smb/client/smb2inode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index e49d0c25eb03..ab069d5285c5 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -941,7 +941,8 @@ int smb2_query_path_info(const unsigned int xid,
 		if (rc || !data->reparse_point)
 			goto out;
 
-		cmds[num_cmds++] = SMB2_OP_QUERY_WSL_EA;
+		if (!tcon->posix_extensions)
+			cmds[num_cmds++] = SMB2_OP_QUERY_WSL_EA;
 		/*
 		 * Skip SMB2_OP_GET_REPARSE if symlink already parsed in create
 		 * response.
-- 
2.47.0


From 371b76b710b14898890253dfbcf405a1f9fdecd8 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Fri, 15 Nov 2024 19:21:04 +0100
Subject: [PATCH 2/3] fs/smb/client: Implement new SMB3 POSIX type

Fixes special files against current Samba.

On the Samba server:

insgesamt 20
131958 brw-r--r--  1 root  root  0, 0 15. Nov 12:04 blockdev
131965 crw-r--r--  1 root  root  1, 1 15. Nov 12:04 chardev
131966 prw-r--r--  1 samba samba    0 15. Nov 12:05 fifo
131953 -rw-rwxrw-+ 2 samba samba    4 18. Nov 11:37 file
131953 -rw-rwxrw-+ 2 samba samba    4 18. Nov 11:37 hardlink
131957 lrwxrwxrwx  1 samba samba    4 15. Nov 12:03 symlink -> file
131954 -rwxrwxr-x+ 1 samba samba    0 18. Nov 15:28 symlinkoversmb

Before:

ls: cannot access '/mnt/smb3unix/posix/blockdev': No data available
ls: cannot access '/mnt/smb3unix/posix/chardev': No data available
ls: cannot access '/mnt/smb3unix/posix/symlinkoversmb': No data available
ls: cannot access '/mnt/smb3unix/posix/fifo': No data available
ls: cannot access '/mnt/smb3unix/posix/symlink': No data available
total 16
     ? -????????? ? ?    ?     ?            ? blockdev
     ? -????????? ? ?    ?     ?            ? chardev
     ? -????????? ? ?    ?     ?            ? fifo
131953 -rw-rwxrw- 2 root samba 4 Nov 18 11:37 file
131953 -rw-rwxrw- 2 root samba 4 Nov 18 11:37 hardlink
     ? -????????? ? ?    ?     ?            ? symlink
     ? -????????? ? ?    ?     ?            ? symlinkoversmb

After:

insgesamt 21
131958 brw-r--r-- 1 root root  0, 0 15. Nov 12:04 blockdev
131965 crw-r--r-- 1 root root  1, 1 15. Nov 12:04 chardev
131966 prw-r--r-- 1 root samba    0 15. Nov 12:05 fifo
131953 -rw-rwxrw- 2 root samba    4 18. Nov 11:37 file
131953 -rw-rwxrw- 2 root samba    4 18. Nov 11:37 hardlink
131957 lrwxrwxrwx 1 root samba    4 15. Nov 12:03 symlink -> file
131954 lrwxrwxr-x 1 root samba   23 18. Nov 15:28 symlinkoversmb -> mnt/smb3unix/posix/file

Signed-off-by: Ralph Boehme <slow@samba.org>
---
 fs/smb/client/cifsproto.h |  1 +
 fs/smb/client/inode.c     | 89 +++++++++++++++++++++++++++++++++++----
 fs/smb/client/readdir.c   | 35 +++++++--------
 fs/smb/client/reparse.c   | 84 ++++++++++++++++++++++--------------
 4 files changed, 149 insertions(+), 60 deletions(-)

diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index d312ea9776ce..20ee7feb9c49 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -676,6 +676,7 @@ int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 int cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 		       struct dentry *dentry, struct cifs_tcon *tcon,
 		       const char *full_path, umode_t mode, dev_t dev);
+umode_t wire_mode_to_posix(u32 wire);
 
 #ifdef CONFIG_CIFS_DFS_UPCALL
 static inline int get_dfs_path(const unsigned int xid, struct cifs_ses *ses,
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 72ebd72dd02b..0dcf9e3635ff 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -724,6 +724,84 @@ static int cifs_sfu_mode(struct cifs_fattr *fattr, const unsigned char *path,
 #endif
 }
 
+#define POSIX_TYPE_FILE    0
+#define POSIX_TYPE_DIR     1
+#define POSIX_TYPE_SYMLINK 2
+#define POSIX_TYPE_CHARDEV 3
+#define POSIX_TYPE_BLKDEV  4
+#define POSIX_TYPE_FIFO    5
+#define POSIX_TYPE_SOCKET  6
+
+#define POSIX_X_OTH      0000001
+#define POSIX_W_OTH      0000002
+#define POSIX_R_OTH      0000004
+#define POSIX_X_GRP      0000010
+#define POSIX_W_GRP      0000020
+#define POSIX_R_GRP      0000040
+#define POSIX_X_USR      0000100
+#define POSIX_W_USR      0000200
+#define POSIX_R_USR      0000400
+#define POSIX_STICKY     0001000
+#define POSIX_SET_GID    0002000
+#define POSIX_SET_UID    0004000
+
+#define POSIX_OTH_MASK      0000007
+#define POSIX_GRP_MASK      0000070
+#define POSIX_USR_MASK      0000700
+#define POSIX_PERM_MASK     0000777
+#define POSIX_FILETYPE_MASK 0070000
+
+#define POSIX_FILETYPE_SHIFT 12
+
+static u32 wire_perms_to_posix(u32 wire)
+{
+        u32 mode = 0;
+
+        mode |= (wire & POSIX_X_OTH) ? S_IXOTH : 0;
+        mode |= (wire & POSIX_W_OTH) ? S_IWOTH : 0;
+        mode |= (wire & POSIX_R_OTH) ? S_IROTH : 0;
+        mode |= (wire & POSIX_X_GRP) ? S_IXGRP : 0;
+        mode |= (wire & POSIX_W_GRP) ? S_IWGRP : 0;
+        mode |= (wire & POSIX_R_GRP) ? S_IRGRP : 0;
+        mode |= (wire & POSIX_X_USR) ? S_IXUSR : 0;
+        mode |= (wire & POSIX_W_USR) ? S_IWUSR : 0;
+        mode |= (wire & POSIX_R_USR) ? S_IRUSR : 0;
+        mode |= (wire & POSIX_STICKY) ? S_ISVTX : 0;
+        mode |= (wire & POSIX_SET_GID) ? S_ISGID : 0;
+        mode |= (wire & POSIX_SET_UID) ? S_ISUID : 0;
+
+        return mode;
+}
+
+static u32 posix_filetypes[] = {
+	S_IFREG,
+	S_IFDIR,
+	S_IFLNK,
+	S_IFCHR,
+	S_IFBLK,
+	S_IFIFO,
+	S_IFSOCK
+};
+
+static u32 wire_filetype_to_posix(u32 wire_type)
+{
+	if (wire_type >= ARRAY_SIZE(posix_filetypes)) {
+		pr_warn("Unexpected type %u", wire_type);
+		return 0;
+	}
+	return posix_filetypes[wire_type];
+}
+
+umode_t wire_mode_to_posix(u32 wire)
+{
+	u32 wire_type;
+	u32 mode;
+
+	wire_type = (wire & POSIX_FILETYPE_MASK) >> POSIX_FILETYPE_SHIFT;
+	mode = (wire_perms_to_posix(wire) | wire_filetype_to_posix(wire_type));
+	return (umode_t)mode;
+}
+
 /* Fill a cifs_fattr struct with info from POSIX info struct */
 static void smb311_posix_info_to_fattr(struct cifs_fattr *fattr,
 				       struct cifs_open_info_data *data,
@@ -760,20 +838,13 @@ static void smb311_posix_info_to_fattr(struct cifs_fattr *fattr,
 	fattr->cf_bytes = le64_to_cpu(info->AllocationSize);
 	fattr->cf_createtime = le64_to_cpu(info->CreationTime);
 	fattr->cf_nlink = le32_to_cpu(info->HardLinks);
-	fattr->cf_mode = (umode_t) le32_to_cpu(info->Mode);
+	fattr->cf_mode = wire_mode_to_posix(le32_to_cpu(info->Mode));
 
 	if (cifs_open_data_reparse(data) &&
 	    cifs_reparse_point_to_fattr(cifs_sb, fattr, data))
 		goto out_reparse;
 
-	fattr->cf_mode &= ~S_IFMT;
-	if (fattr->cf_cifsattrs & ATTR_DIRECTORY) {
-		fattr->cf_mode |= S_IFDIR;
-		fattr->cf_dtype = DT_DIR;
-	} else { /* file */
-		fattr->cf_mode |= S_IFREG;
-		fattr->cf_dtype = DT_REG;
-	}
+	fattr->cf_dtype = S_DT(fattr->cf_mode);
 
 out_reparse:
 	if (S_ISLNK(fattr->cf_mode)) {
diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index b3a8f9c6fcff..28071b2be88c 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -241,31 +241,28 @@ cifs_posix_to_fattr(struct cifs_fattr *fattr, struct smb2_posix_info *info,
 	fattr->cf_nlink = le32_to_cpu(info->HardLinks);
 	fattr->cf_cifsattrs = le32_to_cpu(info->DosAttributes);
 
-	/*
-	 * Since we set the inode type below we need to mask off
-	 * to avoid strange results if bits set above.
-	 * XXX: why not make server&client use the type bits?
-	 */
-	fattr->cf_mode = le32_to_cpu(info->Mode) & ~S_IFMT;
+	if (fattr->cf_cifsattrs & ATTR_REPARSE) {
+		fattr->cf_cifstag = le32_to_cpu(info->ReparseTag);
+	}
+
+	fattr->cf_mode = wire_mode_to_posix(le32_to_cpu(info->Mode));
+	fattr->cf_dtype = S_DT(le32_to_cpu(info->Mode));
+
+	switch (fattr->cf_mode & S_IFMT) {
+	case S_IFLNK:
+	case S_IFBLK:
+	case S_IFCHR:
+		fattr->cf_flags |= CIFS_FATTR_NEED_REVAL;
+		break;
+	default:
+		break;
+	}
 
 	cifs_dbg(FYI, "posix fattr: dev %d, reparse %d, mode %o\n",
 		 le32_to_cpu(info->DeviceId),
 		 le32_to_cpu(info->ReparseTag),
 		 le32_to_cpu(info->Mode));
 
-	if (fattr->cf_cifsattrs & ATTR_DIRECTORY) {
-		fattr->cf_mode |= S_IFDIR;
-		fattr->cf_dtype = DT_DIR;
-	} else {
-		/*
-		 * mark anything that is not a dir as regular
-		 * file. special files should have the REPARSE
-		 * attribute and will be marked as needing revaluation
-		 */
-		fattr->cf_mode |= S_IFREG;
-		fattr->cf_dtype = DT_REG;
-	}
-
 	sid_to_id(cifs_sb, &parsed.owner, fattr, SIDOWNER);
 	sid_to_id(cifs_sb, &parsed.group, fattr, SIDGROUP);
 }
diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index 74abbdf5026c..d392b9c53e43 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -661,44 +661,60 @@ static void wsl_to_fattr(struct cifs_open_info_data *data,
 	fattr->cf_dtype = S_DT(fattr->cf_mode);
 }
 
-bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
-				 struct cifs_fattr *fattr,
-				 struct cifs_open_info_data *data)
+static bool posix_reparse_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;
-	u32 tag = data->reparse.tag;
 
-	if (tag == IO_REPARSE_TAG_NFS && buf) {
-		if (le16_to_cpu(buf->ReparseDataLength) < sizeof(buf->InodeType))
+	if (buf == NULL) {
+		return true;
+	}
+
+	if (le16_to_cpu(buf->ReparseDataLength) < sizeof(buf->InodeType)) {
+		WARN_ON_ONCE(1);
+		return false;
+	}
+
+	switch (le64_to_cpu(buf->InodeType)) {
+	case NFS_SPECFILE_CHR:
+		if (le16_to_cpu(buf->ReparseDataLength) != sizeof(buf->InodeType) + 8) {
+			WARN_ON_ONCE(1);
 			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;
-		case NFS_SPECFILE_FIFO:
-			fattr->cf_mode |= S_IFIFO;
-			break;
-		case NFS_SPECFILE_SOCK:
-			fattr->cf_mode |= S_IFSOCK;
-			break;
-		case NFS_SPECFILE_LNK:
-			fattr->cf_mode |= S_IFLNK;
-			break;
-		default:
+		}
+		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) {
 			WARN_ON_ONCE(1);
 			return false;
 		}
-		goto out;
+		fattr->cf_mode |= S_IFBLK;
+		fattr->cf_rdev = reparse_mkdev(buf->DataBuffer);
+		break;
+	case NFS_SPECFILE_FIFO:
+		fattr->cf_mode |= S_IFIFO;
+		break;
+	case NFS_SPECFILE_SOCK:
+		fattr->cf_mode |= S_IFSOCK;
+		break;
+	case NFS_SPECFILE_LNK:
+		fattr->cf_mode |= S_IFLNK;
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		return false;
 	}
+	return true;
+}
+
+bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
+				 struct cifs_fattr *fattr,
+				 struct cifs_open_info_data *data)
+{
+	u32 tag = data->reparse.tag;
+	bool ok;
 
 	switch (tag) {
 	case IO_REPARSE_TAG_INTERNAL:
@@ -718,15 +734,19 @@ bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
 	case IO_REPARSE_TAG_LX_BLK:
 		wsl_to_fattr(data, cifs_sb, tag, fattr);
 		break;
+	case IO_REPARSE_TAG_NFS:
+		ok = posix_reparse_to_fattr(cifs_sb, fattr, data);
+		if (!ok)
+			return false;
+		break;
 	case 0: /* SMB1 symlink */
 	case IO_REPARSE_TAG_SYMLINK:
-	case IO_REPARSE_TAG_NFS:
 		fattr->cf_mode |= S_IFLNK;
 		break;
 	default:
 		return false;
 	}
-out:
+
 	fattr->cf_dtype = S_DT(fattr->cf_mode);
 	return true;
 }
-- 
2.47.0


From fedaee4c83cb794c7c417a9687c2b1773fb50a65 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Mon, 25 Nov 2024 16:19:56 +0100
Subject: [PATCH 3/3] fs/smb/client: cifs_prime_dcache() for SMB3 POSIX reparse
 points

Signed-off-by: Ralph Boehme <slow@samba.org>
---
 fs/smb/client/readdir.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index 28071b2be88c..d2137bcb8d87 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -71,6 +71,8 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name,
 	struct inode *inode;
 	struct super_block *sb = parent->d_sb;
 	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
+	bool posix = cifs_sb_master_tcon(cifs_sb)->posix_extensions;
+	bool reparse_need_reval = false;
 	DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
 	int rc;
 
@@ -85,7 +87,21 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name,
 		 * this spares us an invalidation.
 		 */
 retry:
-		if ((fattr->cf_cifsattrs & ATTR_REPARSE) ||
+		if (posix) {
+			switch (fattr->cf_mode & S_IFMT) {
+			case S_IFLNK:
+			case S_IFBLK:
+			case S_IFCHR:
+				reparse_need_reval = true;
+				break;
+			default:
+				break;
+			}
+		} else if (fattr->cf_cifsattrs & ATTR_REPARSE) {
+			reparse_need_reval = true;
+		}
+
+		if (reparse_need_reval ||
 		    (fattr->cf_flags & CIFS_FATTR_NEED_REVAL))
 			return;
 
-- 
2.47.0


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

Re: Special files broken against Samba master

[Steve French]

Looks like a big improvement - fewer roundtrips to Samba.  Haven't
tried to Windows yet, but it does look like it breaks mounts to ksmbd
so I may need a minor change to cifs.ko or ksmbd.

On Tue, Dec 3, 2024 at 4:04 AM Ralph Boehme <slow@samba.org> wrote:
>
> On 12/2/24 11:40 PM, Paulo Alcantara wrote:
> > Ralph Boehme <slow@samba.org> writes:
> >> I'm sure I'm doing things wrong. Can you help me getting this across the
> >> line?
> >
> > Patches look good, thanks.  It would be great making sure it didn't
> > regress against NFS server on Windows, but Steve can do that.
>
> great, thanks for taking a look! I was expecting I was likely doing some
> stupid beginner mistakes, but if you think the changes look good, here's
> a v2 with just my +1 added, without code changes.
>
> Thanks!
> -slow



-- 
Thanks,

Steve

Re: Special files broken against Samba master

[Ralph Boehme]

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

Hi Steve!

Thanks for testing! ksmbd needs to be updated to implement the current 
spec. The next major Samba version 4.22 will ship this stuff, so I'd be 
good if cifs.ko follows along asap.

Thanks!
-slow

On 12/3/24 8:56 PM, Steve French wrote:
> Looks like a big improvement - fewer roundtrips to Samba.  Haven't
> tried to Windows yet, but it does look like it breaks mounts to ksmbd
> so I may need a minor change to cifs.ko or ksmbd.
> 
> On Tue, Dec 3, 2024 at 4:04 AM Ralph Boehme <slow@samba.org> wrote:
>>
>> On 12/2/24 11:40 PM, Paulo Alcantara wrote:
>>> Ralph Boehme <slow@samba.org> writes:
>>>> I'm sure I'm doing things wrong. Can you help me getting this across the
>>>> line?
>>>
>>> Patches look good, thanks.  It would be great making sure it didn't
>>> regress against NFS server on Windows, but Steve can do that.
>>
>> great, thanks for taking a look! I was expecting I was likely doing some
>> stupid beginner mistakes, but if you think the changes look good, here's
>> a v2 with just my +1 added, without code changes.
>>
>> Thanks!
>> -slow
> 
> 
> 


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

Re: Special files broken against Samba master

[Steve French]

[-- Attachment #1: Type: text/plain, Size: 1410 bytes --]

I updated patch 2
(0002-fs-smb-client-Implement-new-SMB3-POSIX-type.patch) to address
various checkpatch warnings, and added one small changeset to fix
mounts to older servers which don't fill in the file type in the level
100 Mode field (0004-smb3.1.1-fix-posix-mounts-to-older-servers.patch),
and updated them in cifs-2.6.git for-next.  Let me know if any
objections.  It looks like it does make a big improvement in reporting
special files when mounted to Samba, but want to figure out why the
dentry cache (revalidate call) still requires the extra roundtrip per
file when doing "ls -l" (there may be a way that this enhanced query
dir output could be recognized as still valid, and reduce need for
extra queries).

See attached.

On Tue, Dec 3, 2024 at 4:04 AM Ralph Boehme <slow@samba.org> wrote:
>
> On 12/2/24 11:40 PM, Paulo Alcantara wrote:
> > Ralph Boehme <slow@samba.org> writes:
> >> I'm sure I'm doing things wrong. Can you help me getting this across the
> >> line?
> >
> > Patches look good, thanks.  It would be great making sure it didn't
> > regress against NFS server on Windows, but Steve can do that.
>
> great, thanks for taking a look! I was expecting I was likely doing some
> stupid beginner mistakes, but if you think the changes look good, here's
> a v2 with just my +1 added, without code changes.
>
> Thanks!
> -slow



-- 
Thanks,

Steve

[-- Attachment #2: 0004-smb3.1.1-fix-posix-mounts-to-older-servers.patch --]
[-- Type: text/x-patch, Size: 3500 bytes --]

From 28ab8ec89b756453194c654e3a18cfe1728b0309 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Wed, 4 Dec 2024 17:46:00 -0600
Subject: [PATCH 4/4] smb3.1.1: fix posix mounts to older servers

Some servers which implement the SMB3.1.1 POSIX extensions did not
set the file type in the mode in the infolevel 100 response.
With recent changes for checking the file type via the mode field,
this can cause the root directory to be reported incorrectly and
mounts (e.g. to ksmbd) to fail.

Fixes: 6a832bc8bbb2 ("fs/smb/client: Implement new SMB3 POSIX type")
Cc: stable@vger.kernel.org
Cc: Ralph Boehme <slow@samba.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/smb/client/cifsproto.h |  2 +-
 fs/smb/client/inode.c     | 11 ++++++++---
 fs/smb/client/readdir.c   |  3 ++-
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index 90e3297ea847..754417cb3294 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -669,7 +669,7 @@ int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 int cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 		       struct dentry *dentry, struct cifs_tcon *tcon,
 		       const char *full_path, umode_t mode, dev_t dev);
-umode_t wire_mode_to_posix(u32 wire);
+umode_t wire_mode_to_posix(u32 wire, bool is_dir);
 
 #ifdef CONFIG_CIFS_DFS_UPCALL
 static inline int get_dfs_path(const unsigned int xid, struct cifs_ses *ses,
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index ef913d65c67f..668098d3b108 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -814,13 +814,17 @@ static u32 wire_filetype_to_posix(u32 wire_type)
 	return posix_filetypes[wire_type];
 }
 
-umode_t wire_mode_to_posix(u32 wire)
+umode_t wire_mode_to_posix(u32 wire, bool is_dir)
 {
 	u32 wire_type;
 	u32 mode;
 
 	wire_type = (wire & POSIX_FILETYPE_MASK) >> POSIX_FILETYPE_SHIFT;
-	mode = (wire_perms_to_posix(wire) | wire_filetype_to_posix(wire_type));
+	/* older servers do not set POSIX file type in the mode field in the response */
+	if ((wire_type == 0) && is_dir)
+		mode = wire_perms_to_posix(wire) | S_IFDIR;
+	else
+		mode = (wire_perms_to_posix(wire) | wire_filetype_to_posix(wire_type));
 	return (umode_t)mode;
 }
 
@@ -860,7 +864,8 @@ static void smb311_posix_info_to_fattr(struct cifs_fattr *fattr,
 	fattr->cf_bytes = le64_to_cpu(info->AllocationSize);
 	fattr->cf_createtime = le64_to_cpu(info->CreationTime);
 	fattr->cf_nlink = le32_to_cpu(info->HardLinks);
-	fattr->cf_mode = wire_mode_to_posix(le32_to_cpu(info->Mode));
+	fattr->cf_mode = wire_mode_to_posix(le32_to_cpu(info->Mode),
+					    fattr->cf_cifsattrs & ATTR_DIRECTORY);
 
 	if (cifs_open_data_reparse(data) &&
 	    cifs_reparse_point_to_fattr(cifs_sb, fattr, data))
diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index b906bf78af1e..273358d20a46 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -261,7 +261,8 @@ cifs_posix_to_fattr(struct cifs_fattr *fattr, struct smb2_posix_info *info,
 		fattr->cf_cifstag = le32_to_cpu(info->ReparseTag);
 
 	/* The Mode field in the response can now include the file type as well */
-	fattr->cf_mode = wire_mode_to_posix(le32_to_cpu(info->Mode));
+	fattr->cf_mode = wire_mode_to_posix(le32_to_cpu(info->Mode),
+					    fattr->cf_cifsattrs & ATTR_DIRECTORY);
 	fattr->cf_dtype = S_DT(le32_to_cpu(info->Mode));
 
 	switch (fattr->cf_mode & S_IFMT) {
-- 
2.43.0


[-- Attachment #3: 0001-fs-smb-client-avoid-querying-SMB2_OP_QUERY_WSL_EA-fo.patch --]
[-- Type: text/x-patch, Size: 1038 bytes --]

From ca4b2c4607433033e9c4f4659f809af4261d8992 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Fri, 15 Nov 2024 13:15:50 +0100
Subject: [PATCH 1/4] fs/smb/client: avoid querying SMB2_OP_QUERY_WSL_EA for
 SMB3 POSIX

Avoid extra roundtrip

Cc: stable@vger.kernel.org
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/smb/client/smb2inode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index a188908914fe..a55f0044d30b 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -943,7 +943,8 @@ int smb2_query_path_info(const unsigned int xid,
 		if (rc || !data->reparse_point)
 			goto out;
 
-		cmds[num_cmds++] = SMB2_OP_QUERY_WSL_EA;
+		if (!tcon->posix_extensions)
+			cmds[num_cmds++] = SMB2_OP_QUERY_WSL_EA;
 		/*
 		 * Skip SMB2_OP_GET_REPARSE if symlink already parsed in create
 		 * response.
-- 
2.43.0


[-- Attachment #4: 0002-fs-smb-client-Implement-new-SMB3-POSIX-type.patch --]
[-- Type: text/x-patch, Size: 11126 bytes --]

From 6a832bc8bbb22350f7ffe6ecb2d36f261bb96023 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Fri, 15 Nov 2024 19:21:04 +0100
Subject: [PATCH 2/4] fs/smb/client: Implement new SMB3 POSIX type

Fixes special files against current Samba.

On the Samba server:

insgesamt 20
131958 brw-r--r--  1 root  root  0, 0 15. Nov 12:04 blockdev
131965 crw-r--r--  1 root  root  1, 1 15. Nov 12:04 chardev
131966 prw-r--r--  1 samba samba    0 15. Nov 12:05 fifo
131953 -rw-rwxrw-+ 2 samba samba    4 18. Nov 11:37 file
131953 -rw-rwxrw-+ 2 samba samba    4 18. Nov 11:37 hardlink
131957 lrwxrwxrwx  1 samba samba    4 15. Nov 12:03 symlink -> file
131954 -rwxrwxr-x+ 1 samba samba    0 18. Nov 15:28 symlinkoversmb

Before:

ls: cannot access '/mnt/smb3unix/posix/blockdev': No data available
ls: cannot access '/mnt/smb3unix/posix/chardev': No data available
ls: cannot access '/mnt/smb3unix/posix/symlinkoversmb': No data available
ls: cannot access '/mnt/smb3unix/posix/fifo': No data available
ls: cannot access '/mnt/smb3unix/posix/symlink': No data available
total 16
     ? -????????? ? ?    ?     ?            ? blockdev
     ? -????????? ? ?    ?     ?            ? chardev
     ? -????????? ? ?    ?     ?            ? fifo
131953 -rw-rwxrw- 2 root samba 4 Nov 18 11:37 file
131953 -rw-rwxrw- 2 root samba 4 Nov 18 11:37 hardlink
     ? -????????? ? ?    ?     ?            ? symlink
     ? -????????? ? ?    ?     ?            ? symlinkoversmb

After:

insgesamt 21
131958 brw-r--r-- 1 root root  0, 0 15. Nov 12:04 blockdev
131965 crw-r--r-- 1 root root  1, 1 15. Nov 12:04 chardev
131966 prw-r--r-- 1 root samba    0 15. Nov 12:05 fifo
131953 -rw-rwxrw- 2 root samba    4 18. Nov 11:37 file
131953 -rw-rwxrw- 2 root samba    4 18. Nov 11:37 hardlink
131957 lrwxrwxrwx 1 root samba    4 15. Nov 12:03 symlink -> file
131954 lrwxrwxr-x 1 root samba   23 18. Nov 15:28 symlinkoversmb -> mnt/smb3unix/posix/file

Cc: stable@vger.kernel.org
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/smb/client/cifsproto.h |  1 +
 fs/smb/client/inode.c     | 89 +++++++++++++++++++++++++++++++++++----
 fs/smb/client/readdir.c   | 35 +++++++--------
 fs/smb/client/reparse.c   | 84 ++++++++++++++++++++++--------------
 4 files changed, 149 insertions(+), 60 deletions(-)

diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index bbaaf16af20f..90e3297ea847 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -669,6 +669,7 @@ int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 int cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 		       struct dentry *dentry, struct cifs_tcon *tcon,
 		       const char *full_path, umode_t mode, dev_t dev);
+umode_t wire_mode_to_posix(u32 wire);
 
 #ifdef CONFIG_CIFS_DFS_UPCALL
 static inline int get_dfs_path(const unsigned int xid, struct cifs_ses *ses,
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 42c030687918..ef913d65c67f 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -746,6 +746,84 @@ static int cifs_sfu_mode(struct cifs_fattr *fattr, const unsigned char *path,
 #endif
 }
 
+#define POSIX_TYPE_FILE    0
+#define POSIX_TYPE_DIR     1
+#define POSIX_TYPE_SYMLINK 2
+#define POSIX_TYPE_CHARDEV 3
+#define POSIX_TYPE_BLKDEV  4
+#define POSIX_TYPE_FIFO    5
+#define POSIX_TYPE_SOCKET  6
+
+#define POSIX_X_OTH      0000001
+#define POSIX_W_OTH      0000002
+#define POSIX_R_OTH      0000004
+#define POSIX_X_GRP      0000010
+#define POSIX_W_GRP      0000020
+#define POSIX_R_GRP      0000040
+#define POSIX_X_USR      0000100
+#define POSIX_W_USR      0000200
+#define POSIX_R_USR      0000400
+#define POSIX_STICKY     0001000
+#define POSIX_SET_GID    0002000
+#define POSIX_SET_UID    0004000
+
+#define POSIX_OTH_MASK      0000007
+#define POSIX_GRP_MASK      0000070
+#define POSIX_USR_MASK      0000700
+#define POSIX_PERM_MASK     0000777
+#define POSIX_FILETYPE_MASK 0070000
+
+#define POSIX_FILETYPE_SHIFT 12
+
+static u32 wire_perms_to_posix(u32 wire)
+{
+	u32 mode = 0;
+
+	mode |= (wire & POSIX_X_OTH) ? S_IXOTH : 0;
+	mode |= (wire & POSIX_W_OTH) ? S_IWOTH : 0;
+	mode |= (wire & POSIX_R_OTH) ? S_IROTH : 0;
+	mode |= (wire & POSIX_X_GRP) ? S_IXGRP : 0;
+	mode |= (wire & POSIX_W_GRP) ? S_IWGRP : 0;
+	mode |= (wire & POSIX_R_GRP) ? S_IRGRP : 0;
+	mode |= (wire & POSIX_X_USR) ? S_IXUSR : 0;
+	mode |= (wire & POSIX_W_USR) ? S_IWUSR : 0;
+	mode |= (wire & POSIX_R_USR) ? S_IRUSR : 0;
+	mode |= (wire & POSIX_STICKY) ? S_ISVTX : 0;
+	mode |= (wire & POSIX_SET_GID) ? S_ISGID : 0;
+	mode |= (wire & POSIX_SET_UID) ? S_ISUID : 0;
+
+	return mode;
+}
+
+static u32 posix_filetypes[] = {
+	S_IFREG,
+	S_IFDIR,
+	S_IFLNK,
+	S_IFCHR,
+	S_IFBLK,
+	S_IFIFO,
+	S_IFSOCK
+};
+
+static u32 wire_filetype_to_posix(u32 wire_type)
+{
+	if (wire_type >= ARRAY_SIZE(posix_filetypes)) {
+		pr_warn("Unexpected type %u", wire_type);
+		return 0;
+	}
+	return posix_filetypes[wire_type];
+}
+
+umode_t wire_mode_to_posix(u32 wire)
+{
+	u32 wire_type;
+	u32 mode;
+
+	wire_type = (wire & POSIX_FILETYPE_MASK) >> POSIX_FILETYPE_SHIFT;
+	mode = (wire_perms_to_posix(wire) | wire_filetype_to_posix(wire_type));
+	return (umode_t)mode;
+}
+
 /* Fill a cifs_fattr struct with info from POSIX info struct */
 static void smb311_posix_info_to_fattr(struct cifs_fattr *fattr,
 				       struct cifs_open_info_data *data,
@@ -782,20 +860,13 @@ static void smb311_posix_info_to_fattr(struct cifs_fattr *fattr,
 	fattr->cf_bytes = le64_to_cpu(info->AllocationSize);
 	fattr->cf_createtime = le64_to_cpu(info->CreationTime);
 	fattr->cf_nlink = le32_to_cpu(info->HardLinks);
-	fattr->cf_mode = (umode_t) le32_to_cpu(info->Mode);
+	fattr->cf_mode = wire_mode_to_posix(le32_to_cpu(info->Mode));
 
 	if (cifs_open_data_reparse(data) &&
 	    cifs_reparse_point_to_fattr(cifs_sb, fattr, data))
 		goto out_reparse;
 
-	fattr->cf_mode &= ~S_IFMT;
-	if (fattr->cf_cifsattrs & ATTR_DIRECTORY) {
-		fattr->cf_mode |= S_IFDIR;
-		fattr->cf_dtype = DT_DIR;
-	} else { /* file */
-		fattr->cf_mode |= S_IFREG;
-		fattr->cf_dtype = DT_REG;
-	}
+	fattr->cf_dtype = S_DT(fattr->cf_mode);
 
 out_reparse:
 	if (S_ISLNK(fattr->cf_mode)) {
diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index b3a8f9c6fcff..f878bcdbd319 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -241,31 +241,28 @@ cifs_posix_to_fattr(struct cifs_fattr *fattr, struct smb2_posix_info *info,
 	fattr->cf_nlink = le32_to_cpu(info->HardLinks);
 	fattr->cf_cifsattrs = le32_to_cpu(info->DosAttributes);
 
-	/*
-	 * Since we set the inode type below we need to mask off
-	 * to avoid strange results if bits set above.
-	 * XXX: why not make server&client use the type bits?
-	 */
-	fattr->cf_mode = le32_to_cpu(info->Mode) & ~S_IFMT;
+	if (fattr->cf_cifsattrs & ATTR_REPARSE)
+		fattr->cf_cifstag = le32_to_cpu(info->ReparseTag);
+
+	/* The Mode field in the response can now include the file type as well */
+	fattr->cf_mode = wire_mode_to_posix(le32_to_cpu(info->Mode));
+	fattr->cf_dtype = S_DT(le32_to_cpu(info->Mode));
+
+	switch (fattr->cf_mode & S_IFMT) {
+	case S_IFLNK:
+	case S_IFBLK:
+	case S_IFCHR:
+		fattr->cf_flags |= CIFS_FATTR_NEED_REVAL;
+		break;
+	default:
+		break;
+	}
 
 	cifs_dbg(FYI, "posix fattr: dev %d, reparse %d, mode %o\n",
 		 le32_to_cpu(info->DeviceId),
 		 le32_to_cpu(info->ReparseTag),
 		 le32_to_cpu(info->Mode));
 
-	if (fattr->cf_cifsattrs & ATTR_DIRECTORY) {
-		fattr->cf_mode |= S_IFDIR;
-		fattr->cf_dtype = DT_DIR;
-	} else {
-		/*
-		 * mark anything that is not a dir as regular
-		 * file. special files should have the REPARSE
-		 * attribute and will be marked as needing revaluation
-		 */
-		fattr->cf_mode |= S_IFREG;
-		fattr->cf_dtype = DT_REG;
-	}
-
 	sid_to_id(cifs_sb, &parsed.owner, fattr, SIDOWNER);
 	sid_to_id(cifs_sb, &parsed.group, fattr, SIDGROUP);
 }
diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index e81d2d78ddb7..50b1aba20008 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -803,44 +803,60 @@ static void wsl_to_fattr(struct cifs_open_info_data *data,
 	fattr->cf_dtype = S_DT(fattr->cf_mode);
 }
 
-bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
-				 struct cifs_fattr *fattr,
-				 struct cifs_open_info_data *data)
+static bool posix_reparse_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;
-	u32 tag = data->reparse.tag;
 
-	if (tag == IO_REPARSE_TAG_NFS && buf) {
-		if (le16_to_cpu(buf->ReparseDataLength) < sizeof(buf->InodeType))
+
+	if (buf == NULL)
+		return true;
+
+	if (le16_to_cpu(buf->ReparseDataLength) < sizeof(buf->InodeType)) {
+		WARN_ON_ONCE(1);
+		return false;
+	}
+
+	switch (le64_to_cpu(buf->InodeType)) {
+	case NFS_SPECFILE_CHR:
+		if (le16_to_cpu(buf->ReparseDataLength) != sizeof(buf->InodeType) + 8) {
+			WARN_ON_ONCE(1);
 			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;
-		case NFS_SPECFILE_FIFO:
-			fattr->cf_mode |= S_IFIFO;
-			break;
-		case NFS_SPECFILE_SOCK:
-			fattr->cf_mode |= S_IFSOCK;
-			break;
-		case NFS_SPECFILE_LNK:
-			fattr->cf_mode |= S_IFLNK;
-			break;
-		default:
+		}
+		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) {
 			WARN_ON_ONCE(1);
 			return false;
 		}
-		goto out;
+		fattr->cf_mode |= S_IFBLK;
+		fattr->cf_rdev = reparse_mkdev(buf->DataBuffer);
+		break;
+	case NFS_SPECFILE_FIFO:
+		fattr->cf_mode |= S_IFIFO;
+		break;
+	case NFS_SPECFILE_SOCK:
+		fattr->cf_mode |= S_IFSOCK;
+		break;
+	case NFS_SPECFILE_LNK:
+		fattr->cf_mode |= S_IFLNK;
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		return false;
 	}
+	return true;
+}
+
+bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
+				 struct cifs_fattr *fattr,
+				 struct cifs_open_info_data *data)
+{
+	u32 tag = data->reparse.tag;
+	bool ok;
 
 	switch (tag) {
 	case IO_REPARSE_TAG_INTERNAL:
@@ -860,15 +876,19 @@ bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb,
 	case IO_REPARSE_TAG_LX_BLK:
 		wsl_to_fattr(data, cifs_sb, tag, fattr);
 		break;
+	case IO_REPARSE_TAG_NFS:
+		ok = posix_reparse_to_fattr(cifs_sb, fattr, data);
+		if (!ok)
+			return false;
+		break;
 	case 0: /* SMB1 symlink */
 	case IO_REPARSE_TAG_SYMLINK:
-	case IO_REPARSE_TAG_NFS:
 		fattr->cf_mode |= S_IFLNK;
 		break;
 	default:
 		return false;
 	}
-out:
+
 	fattr->cf_dtype = S_DT(fattr->cf_mode);
 	return true;
 }
-- 
2.43.0


[-- Attachment #5: 0003-fs-smb-client-cifs_prime_dcache-for-SMB3-POSIX-repar.patch --]
[-- Type: text/x-patch, Size: 1612 bytes --]

From 8cb0bc5436351de8a11eef13b7367d64cc0d6c68 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Mon, 25 Nov 2024 16:19:56 +0100
Subject: [PATCH 3/4] fs/smb/client: cifs_prime_dcache() for SMB3 POSIX reparse
 points

Spares an extra revalidation request

Cc: stable@vger.kernel.org
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/smb/client/readdir.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index f878bcdbd319..b906bf78af1e 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -71,6 +71,8 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name,
 	struct inode *inode;
 	struct super_block *sb = parent->d_sb;
 	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
+	bool posix = cifs_sb_master_tcon(cifs_sb)->posix_extensions;
+	bool reparse_need_reval = false;
 	DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
 	int rc;
 
@@ -85,7 +87,21 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name,
 		 * this spares us an invalidation.
 		 */
 retry:
-		if ((fattr->cf_cifsattrs & ATTR_REPARSE) ||
+		if (posix) {
+			switch (fattr->cf_mode & S_IFMT) {
+			case S_IFLNK:
+			case S_IFBLK:
+			case S_IFCHR:
+				reparse_need_reval = true;
+				break;
+			default:
+				break;
+			}
+		} else if (fattr->cf_cifsattrs & ATTR_REPARSE) {
+			reparse_need_reval = true;
+		}
+
+		if (reparse_need_reval ||
 		    (fattr->cf_flags & CIFS_FATTR_NEED_REVAL))
 			return;
 
-- 
2.43.0

Re: Special files broken against Samba master

[Ralph Boehme]

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

Hi Steve

thanks for digging into this!

On 12/5/24 1:06 AM, Steve French wrote:
> I updated patch 2
> (0002-fs-smb-client-Implement-new-SMB3-POSIX-type.patch) to address
> various checkpatch warnings,

oh, sorry for those! I'll try to remember to run it myself next time.

> and added one small changeset to fix
> mounts to older servers which don't fill in the file type in the level
> 100 Mode field (0004-smb3.1.1-fix-posix-mounts-to-older-servers.patch),
> and updated them in cifs-2.6.git for-next.  Let me know if any
> objections.

hm, not sure I like adding code to support non compliant SMB3 POSIX 
server implementations at this early stage of implementing them across 
various projects, clients and servers. Can't Namjae just fix it in ksmbd?

> It looks like it does make a big improvement in reporting
> special files when mounted to Samba, but want to figure out why the
> dentry cache (revalidate call) still requires the extra roundtrip per
> file when doing "ls -l" (there may be a way that this enhanced query
> dir output could be recognized as still valid, and reduce need for
> extra queries).

it should only do an extra roundtrip when needed for symlinks, character 
and block devices as for those we wee need additional info. But for 
fifos and sockets the SMB2-FIND POSIX-infolevel is sufficient and in my 
testing I didn't see extra roundtrips for those.

Thanks!
-slow

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

Re: Special files broken against Samba master

[Steve French]

On Fri, Dec 6, 2024 at 6:11 AM Ralph Boehme <slow@samba.org> wrote:
>
> Hi Steve
>
> thanks for digging into this!
>
> On 12/5/24 1:06 AM, Steve French wrote:
> > I updated patch 2
> > (0002-fs-smb-client-Implement-new-SMB3-POSIX-type.patch) to address
> > various checkpatch warnings,
>
> oh, sorry for those! I'll try to remember to run it myself next time.
>
> > and added one small changeset to fix
> > mounts to older servers which don't fill in the file type in the level
> > 100 Mode field (0004-smb3.1.1-fix-posix-mounts-to-older-servers.patch),
> > and updated them in cifs-2.6.git for-next.  Let me know if any
> > objections.
>
> hm, not sure I like adding code to support non compliant SMB3 POSIX
> server implementations at this early stage of implementing them across
> various projects, clients and servers. Can't Namjae just fix it in ksmbd?

We will make sure ksmbd is changed to fill in the file type, but we
should reduce risks (where fix is easy) of breaking mounts to existing
servers, especially since this has been in for more than three years and
since the sanity check is safe and easy (ATTRIBUTE_DIRECTORY
will always be set correctly by a server).   There is also at least
one other server that implemented earlier version of the SMB3.1.1
POSIX Extensions so better to be safe when we can do a minor
check like this on the client so we don't risk any regressions, and
it also will reduce risk of a future server bug breaking mounts.


-- 
Thanks,

Steve

Re: Special files broken against Samba master

[Pali Rohár]

Hello, this patch is incomplete and still does not fix the main problem
that SMB2_OP_QUERY_WSL_EA command does not work with any Windows SMB
server except the last Windows Server version. On non-recent Windows
versions it is not possible to set both EAs and reparse point at the
same time and Windows SMB server is returning error when trying to query
EAs on file with reparse point.

Which basically means that it is not possible to query data about the
special files from Windows SMB server (except 2022 version).

More details are in the email which I wrote in September:
https://lore.kernel.org/linux-cifs/20240928140939.vjndryndfngzq7x4@pali/

I proposed similar but extended patch which skips asking for EAs based
on reparse point:
https://lore.kernel.org/linux-cifs/20240913200204.10660-1-pali@kernel.org/

But it was somehow rejected as the proper solution should be different:
https://lore.kernel.org/linux-cifs/20240917210707.4lt4obty7wlmm42j@pali/

And that is why I'm surprised that the batch below was accepted...

Are you planning to implement a proper solution? If not that I would
propose to reconsider my original idea, which will workaround also
Windows SMB server, and not only Samba server.

As I wrote in previous emails, I think that this is a serious issue as
it disallow to use any reparse points against non-recent Windows Server
systems.

On Wednesday 27 November 2024 20:23:29 Ralph Boehme wrote:
> From a57c32da874f285af266b7fbbaefb7940991d049 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow@samba.org>
> Date: Fri, 15 Nov 2024 13:15:50 +0100
> Subject: [PATCH 1/3] fs/smb/client: avoid querying SMB2_OP_QUERY_WSL_EA for
>  SMB3 POSIX
> 
> ---
>  fs/smb/client/smb2inode.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> index e49d0c25eb03..ab069d5285c5 100644
> --- a/fs/smb/client/smb2inode.c
> +++ b/fs/smb/client/smb2inode.c
> @@ -941,7 +941,8 @@ int smb2_query_path_info(const unsigned int xid,
>  		if (rc || !data->reparse_point)
>  			goto out;
>  
> -		cmds[num_cmds++] = SMB2_OP_QUERY_WSL_EA;
> +		if (!tcon->posix_extensions)
> +			cmds[num_cmds++] = SMB2_OP_QUERY_WSL_EA;
>  		/*
>  		 * Skip SMB2_OP_GET_REPARSE if symlink already parsed in create
>  		 * response.
> -- 
> 2.47.0

Re: Special files broken against Samba master

[Paulo Alcantara]

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

> Hello, this patch is incomplete and still does not fix the main problem
> that SMB2_OP_QUERY_WSL_EA command does not work with any Windows SMB
> server except the last Windows Server version. On non-recent Windows
> versions it is not possible to set both EAs and reparse point at the
> same time and Windows SMB server is returning error when trying to query
> EAs on file with reparse point.

No, Ralph's patch has nothing to do with your problem.  SMB3.1.1 POSIX
will support NFS reparse points with no EAs, so makes sense skipping
SMB2_OP_QUERY_WSL_EA altogether for posix case.

> Which basically means that it is not possible to query data about the
> special files from Windows SMB server (except 2022 version).
>
> More details are in the email which I wrote in September:
> https://lore.kernel.org/linux-cifs/20240928140939.vjndryndfngzq7x4@pali/
>
> I proposed similar but extended patch which skips asking for EAs based
> on reparse point:
> https://lore.kernel.org/linux-cifs/20240913200204.10660-1-pali@kernel.org/

Yes, that patch looks incorrect and untested.  Can you tell me how is
@data->reparse.tag supposed to be set, for non-readdir case, if the
compound request wasn't sent yet?  Have you tried to stat(2) those files
with your patch?

> But it was somehow rejected as the proper solution should be different:
> https://lore.kernel.org/linux-cifs/20240917210707.4lt4obty7wlmm42j@pali/

Yes.  Have you sent a patch with the proposed solution yet?

Re: Special files broken against Samba master

[Pali Rohár]

On Monday 09 December 2024 16:28:08 Paulo Alcantara wrote:
> Pali Rohár <pali@kernel.org> writes:
> 
> > Hello, this patch is incomplete and still does not fix the main problem
> > that SMB2_OP_QUERY_WSL_EA command does not work with any Windows SMB
> > server except the last Windows Server version. On non-recent Windows
> > versions it is not possible to set both EAs and reparse point at the
> > same time and Windows SMB server is returning error when trying to query
> > EAs on file with reparse point.
> 
> No, Ralph's patch has nothing to do with your problem.  SMB3.1.1 POSIX
> will support NFS reparse points with no EAs, so makes sense skipping
> SMB2_OP_QUERY_WSL_EA altogether for posix case.

Ok, so this is just a coincidence that skip was added at the same place.
And I though that it is the same thing (which now I see that is not).

> > Which basically means that it is not possible to query data about the
> > special files from Windows SMB server (except 2022 version).
> >
> > More details are in the email which I wrote in September:
> > https://lore.kernel.org/linux-cifs/20240928140939.vjndryndfngzq7x4@pali/
> >
> > I proposed similar but extended patch which skips asking for EAs based
> > on reparse point:
> > https://lore.kernel.org/linux-cifs/20240913200204.10660-1-pali@kernel.org/
> 
> Yes, that patch looks incorrect and untested.  Can you tell me how is
> @data->reparse.tag supposed to be set, for non-readdir case, if the
> compound request wasn't sent yet?  Have you tried to stat(2) those files
> with your patch?

I need to rethink about it. Basically I already dropped this patch as I
was expecting the proper solution.

> > But it was somehow rejected as the proper solution should be different:
> > https://lore.kernel.org/linux-cifs/20240917210707.4lt4obty7wlmm42j@pali/
> 
> Yes.  Have you sent a patch with the proposed solution yet?

I have not sent, I was not working on it. I was in impression that you
are going to implement the proper solution.

Re: Special files broken against Samba master

[Paulo Alcantara]

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

> On Monday 09 December 2024 16:28:08 Paulo Alcantara wrote:
>>
>> Yes.  Have you sent a patch with the proposed solution yet?
>
> I have not sent, I was not working on it. I was in impression that you
> are going to implement the proper solution.

Yes, but priorities have changed, unfortunately.

Re: Special files broken against Samba master

[Pali Rohár]

On Monday 09 December 2024 16:42:54 Paulo Alcantara wrote:
> Pali Rohár <pali@kernel.org> writes:
> 
> > On Monday 09 December 2024 16:28:08 Paulo Alcantara wrote:
> >>
> >> Yes.  Have you sent a patch with the proposed solution yet?
> >
> > I have not sent, I was not working on it. I was in impression that you
> > are going to implement the proper solution.
> 
> Yes, but priorities have changed, unfortunately.

Ok. Would you have a time to prepare a patch? I have still feeling that
the smb2_compound_op() code and EAs code is rather complicated for me.
I would like if somebody else could look at it, as I have feeling if I
try to do it, it can end up with something more broken...

Re: Special files broken against Samba master

[Paulo Alcantara]

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

> Ok. Would you have a time to prepare a patch? I have still feeling that
> the smb2_compound_op() code and EAs code is rather complicated for me.
> I would like if somebody else could look at it, as I have feeling if I
> try to do it, it can end up with something more broken...

Yeah, it's a bit tricky.  I'll let you know when I have patch so I can
try it against your old servers.

Re: Special files broken against Samba master

[Pali Rohár]

On Tuesday 10 December 2024 10:19:26 Paulo Alcantara wrote:
> Pali Rohár <pali@kernel.org> writes:
> 
> > Ok. Would you have a time to prepare a patch? I have still feeling that
> > the smb2_compound_op() code and EAs code is rather complicated for me.
> > I would like if somebody else could look at it, as I have feeling if I
> > try to do it, it can end up with something more broken...
> 
> Yeah, it's a bit tricky.  I'll let you know when I have patch so I can
> try it against your old servers.

I looked at that code and it looks to be really more complicated to
implement partial parsing of the reply, as compound code is bound to
many places and code paths.

Meanwhile I have tried to implement a new version to retry the call
without querying EAs when server returns that EAs on specific paths are
unsupported:
https://patchwork.kernel.org/project/cifs-client/patch/20241224131859.3457-1-pali@kernel.org/

I think that this approach should now work correctly.