Re: Special files broken against Samba master

[Ralph Boehme <slow@samba.org>]

[-- 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 --]