public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] cifs: Improve client SFU support for special files
@ 2024-09-12 12:05 Pali Rohár
  2024-09-12 12:05 ` [PATCH 1/7] cifs: Fix recognizing SFU symlinks Pali Rohár
                   ` (7 more replies)
  0 siblings, 8 replies; 30+ messages in thread
From: Pali Rohár @ 2024-09-12 12:05 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

This patch series implement full support for SFU-style of special files
(fifo, socket, block, char, symlink) and makes client cifs's -o sfu
mount option to be fully compatible with SFU.

Before this patch series, kernel cifs client was able to:
* create new char and block devices in SFU-style, and detect them
* detect existing SFU fifo, but no create/mkfifo SFU functionality
* detect symlink SFU symlink, but no readlink() functionality and
  neither no create SFU symlink functionality

And it was able to create some SFU-incompatible sockets and fifos (when
-o sfu was specified) which were not recognized by neither original MS
SFU implementation and neither by others.

This patch series implements missing functionality, which is:
* detect existing SFU sockets
* create new SFU sockets
* create new SFU fifos
* create new SFU symlink
* readlink() support for existing SFU symlinks

In following pragraphs are some details about these SFU special files
which usage on Linux has to be activated by -o sfu mount option.

SFU-style fifo is empty file with system attribute set. This format is
used by old Microsoft POSIX subsystem and later also by OpenNT/Interix
subsystem (which replaced Microsoft POSIX subsystem and is part of
Microsoft SFU). OpenNT is previous name of Interix versions 1.x/2.x.
Microsoft POSIX subsystem is available since the first Windows NT
version 3.1, and it was replaced by Interix since Windows XP. Interix
continue to use this format up to the its last released version for
Windows 8 / Server 2012 (part of Subsystem for UNIX-based Applications).
Hence these SFU-style fifos are for very long time unified and widely
supported.

SFU-style socket is file which has system attribute set and file content
is one zero byte.

SFU-style symlink is file which has system attribute set and file content
is buffer "IntxLNK\1" (8th byte is 0x01) followed by the target location
encoded in little endian UCS-2/UTF-16. There is no trailing nul.

SFU-style char or block device is file which has system attribute set
and file content is buffer "IntxBLK\0" or "IntxCHR\0" (8th byte is 0x00)
followed by major and minor numbers encoded in twos 64-bit little endian.

Format of SFU-style sockets, symlinks, char and block devices was
introduced in Interix 3.0 subsystem, as part of the Microsoft SFU 3.0
and is used also by all later versions, up to the Windows 8 / Server 2012
(part of Subsystem for UNIX-based Applications) where it was deprecated.
Previous OpenNT/Interix versions had no support for UNIX domain sockets
(and socket files), symlinks or possibility to create character or block
devices (but they had block/char devices in-memory, returned by stat()).

Microsoft NFS server up to the version included in Windows Server 2008 R2
also uses this SFU-style format of special files when storing them on
local filesystem. Later Microsoft NFS server versions (starting in
Windows Server 2012) use new NFS reparse format, which Interix subsystem
(included in SFU or SUA) does not understand.

Even SFU-style of special files is old format, it has one big advantage,
this format does not require any support on SMB/CIFS server of special
files, as everything is stored only in the file content. The only
requirement from server is support for system attribute. So this allows
to store special files also on FAT filesystem.

This patch series makes cifs -o sfu mount option compatible with
SFU-style of special files, and so compatible with the latest SFU/SUA.

Note that -o sfu is by default turned off, so these changes should have
no effect on default cifs mounts.

Manually tested with MS SFU 3.5 (for Windows XP) and MS SUA 6.2 (latest
released version of Interix) that interop works correctly, special files
created by POSIX/Interix application can be recognized by Linux cifs
client (exported over MS SMB) with these patches (and vice-versa setup,
created by Linux cifs client and recognized in POSIX/Interix subsystem).

Manually tested that old Linux 4.19 cifs client version can recognize
SFU-style of special files created by Linux cifs client this patch
series (except socket, which is unsupported in this Linux cifs version).

Patch series is based on the latest upstream tag v6.11-rc7.

Pali Rohár (7):
  cifs: Fix recognizing SFU symlinks
  cifs: Add support for reading SFU symlink location
  cifs: Put explicit zero byte into SFU block/char types
  cifs: Show debug message when SFU Fifo type was detected
  cifs: Recognize SFU socket type
  cifs: Fix creating of SFU fifo and socket special files
  cifs: Add support for creating SFU symlinks

 fs/smb/client/cifspdu.h    |  6 ---
 fs/smb/client/cifsproto.h  |  4 ++
 fs/smb/client/cifssmb.c    |  8 ++--
 fs/smb/client/fs_context.c | 13 ++++---
 fs/smb/client/inode.c      | 42 ++++++++++++++++++--
 fs/smb/client/link.c       |  3 ++
 fs/smb/client/smb1ops.c    |  2 +-
 fs/smb/client/smb2ops.c    | 78 ++++++++++++++++++++++++++++----------
 8 files changed, 117 insertions(+), 39 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/7] cifs: Fix recognizing SFU symlinks
  2024-09-12 12:05 [PATCH 0/7] cifs: Improve client SFU support for special files Pali Rohár
@ 2024-09-12 12:05 ` Pali Rohár
  2024-09-13 20:04   ` Pali Rohár
  2024-09-12 12:05 ` [PATCH 2/7] cifs: Add support for reading SFU symlink location Pali Rohár
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2024-09-12 12:05 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

SFU symlinks have 8 byte prefix: "IntxLNK\1".
So check also the last 8th byte 0x01.

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

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 73e2e6c230b7..7d424e769a56 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -612,7 +612,7 @@ cifs_sfu_type(struct cifs_fattr *fattr, const char *path,
 			cifs_dbg(FYI, "Socket\n");
 			fattr->cf_mode |= S_IFSOCK;
 			fattr->cf_dtype = DT_SOCK;
-		} else if (memcmp("IntxLNK", pbuf, 7) == 0) {
+		} else if (memcmp("IntxLNK\1", pbuf, 8) == 0) {
 			cifs_dbg(FYI, "Symlink\n");
 			fattr->cf_mode |= S_IFLNK;
 			fattr->cf_dtype = DT_LNK;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/7] cifs: Add support for reading SFU symlink location
  2024-09-12 12:05 [PATCH 0/7] cifs: Improve client SFU support for special files Pali Rohár
  2024-09-12 12:05 ` [PATCH 1/7] cifs: Fix recognizing SFU symlinks Pali Rohár
@ 2024-09-12 12:05 ` Pali Rohár
  2024-09-12 12:05 ` [PATCH 3/7] cifs: Put explicit zero byte into SFU block/char types Pali Rohár
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2024-09-12 12:05 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

Currently when sfu mount option is specified then CIFS can recognize SFU
symlink, but is not able to read symlink target location. readlink()
syscall just returns that operation is not supported.

Implement this missing functionality in cifs_sfu_type() function. Read
target location of SFU-style symlink, parse it and fill into fattr's
cf_symlink_target member.

SFU-style symlink is file which has system attribute set and file content
is buffer "IntxLNK\1" (8th byte is 0x01) followed by the target location
encoded in little endian UCS-2/UTF-16. This format was introduced in
Interix 3.0 subsystem, as part of the Microsoft SFU 3.0 and is used also by
all later versions. Previous versions had no symlink support.

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

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 7d424e769a56..e8567ed63f22 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -529,6 +529,8 @@ cifs_sfu_type(struct cifs_fattr *fattr, const char *path,
 	struct cifs_fid fid;
 	struct cifs_open_parms oparms;
 	struct cifs_io_parms io_parms = {0};
+	char *symlink_buf_utf16;
+	unsigned int symlink_len_utf16;
 	char buf[24];
 	unsigned int bytes_read;
 	char *pbuf;
@@ -616,6 +618,33 @@ cifs_sfu_type(struct cifs_fattr *fattr, const char *path,
 			cifs_dbg(FYI, "Symlink\n");
 			fattr->cf_mode |= S_IFLNK;
 			fattr->cf_dtype = DT_LNK;
+			if ((fattr->cf_eof > 8) && (fattr->cf_eof % 2 == 0)) {
+				symlink_buf_utf16 = kmalloc(fattr->cf_eof-8 + 1, GFP_KERNEL);
+				if (symlink_buf_utf16) {
+					io_parms.offset = 8;
+					io_parms.length = fattr->cf_eof-8 + 1;
+					buf_type = CIFS_NO_BUFFER;
+					rc = tcon->ses->server->ops->sync_read(xid, &fid, &io_parms,
+									       &symlink_len_utf16,
+									       &symlink_buf_utf16,
+									       &buf_type);
+					if ((rc == 0) &&
+					    (symlink_len_utf16 > 0) &&
+					    (symlink_len_utf16 < fattr->cf_eof-8 + 1) &&
+					    (symlink_len_utf16 % 2 == 0)) {
+						fattr->cf_symlink_target =
+							cifs_strndup_from_utf16(symlink_buf_utf16,
+										symlink_len_utf16,
+										true,
+										cifs_sb->local_nls);
+						if (!fattr->cf_symlink_target)
+							rc = -ENOMEM;
+					}
+					kfree(symlink_buf_utf16);
+				} else {
+					rc = -ENOMEM;
+				}
+			}
 		} else if (memcmp("LnxFIFO", pbuf, 8) == 0) {
 			cifs_dbg(FYI, "FIFO\n");
 			fattr->cf_mode |= S_IFIFO;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/7] cifs: Put explicit zero byte into SFU block/char types
  2024-09-12 12:05 [PATCH 0/7] cifs: Improve client SFU support for special files Pali Rohár
  2024-09-12 12:05 ` [PATCH 1/7] cifs: Fix recognizing SFU symlinks Pali Rohár
  2024-09-12 12:05 ` [PATCH 2/7] cifs: Add support for reading SFU symlink location Pali Rohár
@ 2024-09-12 12:05 ` Pali Rohár
  2024-09-12 12:05 ` [PATCH 4/7] cifs: Show debug message when SFU Fifo type was detected Pali Rohár
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2024-09-12 12:05 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

SFU types IntxCHR and IntxBLK are 8 bytes with zero as last byte. Make it
explicit in memcpy and memset calls, so the zero byte is visible in the
code (and not hidden as string trailing nul byte).

It is important for reader to show the last byte for block and char types
because it differs from the last byte of symlink type (which has it 0x01).

Also it is important to show that the type is not nul-term string, but
rather 8 bytes (with some printable bytes).

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

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index e8567ed63f22..d8c39989840e 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -586,7 +586,7 @@ cifs_sfu_type(struct cifs_fattr *fattr, const char *path,
 	rc = tcon->ses->server->ops->sync_read(xid, &fid, &io_parms,
 					&bytes_read, &pbuf, &buf_type);
 	if ((rc == 0) && (bytes_read >= 8)) {
-		if (memcmp("IntxBLK", pbuf, 8) == 0) {
+		if (memcmp("IntxBLK\0", pbuf, 8) == 0) {
 			cifs_dbg(FYI, "Block device\n");
 			fattr->cf_mode |= S_IFBLK;
 			fattr->cf_dtype = DT_BLK;
@@ -598,7 +598,7 @@ cifs_sfu_type(struct cifs_fattr *fattr, const char *path,
 				mnr = le64_to_cpu(*(__le64 *)(pbuf+16));
 				fattr->cf_rdev = MKDEV(mjr, mnr);
 			}
-		} else if (memcmp("IntxCHR", pbuf, 8) == 0) {
+		} else if (memcmp("IntxCHR\0", pbuf, 8) == 0) {
 			cifs_dbg(FYI, "Char device\n");
 			fattr->cf_mode |= S_IFCHR;
 			fattr->cf_dtype = DT_CHR;
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index e6540072ffb0..9c2d065d3cc4 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -5072,12 +5072,12 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 
 	switch (mode & S_IFMT) {
 	case S_IFCHR:
-		strscpy(pdev.type, "IntxCHR");
+		memcpy(pdev.type, "IntxCHR\0", 8);
 		pdev.major = cpu_to_le64(MAJOR(dev));
 		pdev.minor = cpu_to_le64(MINOR(dev));
 		break;
 	case S_IFBLK:
-		strscpy(pdev.type, "IntxBLK");
+		memcpy(pdev.type, "IntxBLK\0", 8);
 		pdev.major = cpu_to_le64(MAJOR(dev));
 		pdev.minor = cpu_to_le64(MINOR(dev));
 		break;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 4/7] cifs: Show debug message when SFU Fifo type was detected
  2024-09-12 12:05 [PATCH 0/7] cifs: Improve client SFU support for special files Pali Rohár
                   ` (2 preceding siblings ...)
  2024-09-12 12:05 ` [PATCH 3/7] cifs: Put explicit zero byte into SFU block/char types Pali Rohár
@ 2024-09-12 12:05 ` Pali Rohár
  2024-09-12 12:05 ` [PATCH 5/7] cifs: Recognize SFU socket type Pali Rohár
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2024-09-12 12:05 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

For debugging purposes it is a good idea to show detected SFU type also for
Fifo. Debug message is already print for all other special types.

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

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index d8c39989840e..70c32b40ede0 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -541,6 +541,7 @@ cifs_sfu_type(struct cifs_fattr *fattr, const char *path,
 	fattr->cf_mode &= ~S_IFMT;
 
 	if (fattr->cf_eof == 0) {
+		cifs_dbg(FYI, "Fifo\n");
 		fattr->cf_mode |= S_IFIFO;
 		fattr->cf_dtype = DT_FIFO;
 		return 0;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 5/7] cifs: Recognize SFU socket type
  2024-09-12 12:05 [PATCH 0/7] cifs: Improve client SFU support for special files Pali Rohár
                   ` (3 preceding siblings ...)
  2024-09-12 12:05 ` [PATCH 4/7] cifs: Show debug message when SFU Fifo type was detected Pali Rohár
@ 2024-09-12 12:05 ` Pali Rohár
  2024-09-12 12:05 ` [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files Pali Rohár
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2024-09-12 12:05 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

SFU since its (first) version 3.0 supports AF_LOCAL sockets and stores them
on filesytem as system file with one zero byte. Add support for detecting
this SFU socket type into cifs_sfu_type() function.

With this change cifs_sfu_type() would correctly detect all special file
types created by SFU: fifo, socket, symlink, block and char.

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

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 70c32b40ede0..331a86074ae7 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -545,7 +545,7 @@ cifs_sfu_type(struct cifs_fattr *fattr, const char *path,
 		fattr->cf_mode |= S_IFIFO;
 		fattr->cf_dtype = DT_FIFO;
 		return 0;
-	} else if (fattr->cf_eof < 8) {
+	} else if (fattr->cf_eof > 1 && fattr->cf_eof < 8) {
 		fattr->cf_mode |= S_IFREG;
 		fattr->cf_dtype = DT_REG;
 		return -EINVAL;	 /* EOPNOTSUPP? */
@@ -655,6 +655,10 @@ cifs_sfu_type(struct cifs_fattr *fattr, const char *path,
 			fattr->cf_dtype = DT_REG;
 			rc = -EOPNOTSUPP;
 		}
+	} else if ((rc == 0) && (bytes_read == 1) && (pbuf[0] == '\0')) {
+		cifs_dbg(FYI, "Socket\n");
+		fattr->cf_mode |= S_IFSOCK;
+		fattr->cf_dtype = DT_SOCK;
 	} else {
 		fattr->cf_mode |= S_IFREG; /* then it is a file */
 		fattr->cf_dtype = DT_REG;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files
  2024-09-12 12:05 [PATCH 0/7] cifs: Improve client SFU support for special files Pali Rohár
                   ` (4 preceding siblings ...)
  2024-09-12 12:05 ` [PATCH 5/7] cifs: Recognize SFU socket type Pali Rohár
@ 2024-09-12 12:05 ` Pali Rohár
  2024-09-13 20:07   ` Pali Rohár
  2024-09-15 19:45   ` [PATCH 0/4] cifs: Improve client SFU support for special files (2) Pali Rohár
  2024-09-12 12:05 ` [PATCH 7/7] cifs: Add support for creating SFU symlinks Pali Rohár
  2024-09-13  3:44 ` [PATCH 0/7] cifs: Improve client SFU support for special files Steve French
  7 siblings, 2 replies; 30+ messages in thread
From: Pali Rohár @ 2024-09-12 12:05 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

SFU-style fifo is empty file with system attribute set. This format is used
by old Microsoft POSIX subsystem and later also by OpenNT/Interix subsystem
(which replaced Microsoft POSIX subsystem and is part of Microsoft SFU).

SFU-style socket is file which has system attribute set and file content is
one zero byte. This format was introduced in Interix 3.0 subsystem, as part
of the Microsoft SFU 3.0 and is used also by all later versions. Previous
versions had no UNIX domain socket support.

Currently when sfu mount option is specified then CIFS creates fifo and
socket special files with some strange LnxSOCK or LnxFIFO content which is
not compatible with Microsoft SFU and neither recognized by other SMB
implementations which have some SFU support, including older Linux cifs
implementations.

So when sfu mount option is specified, create all fifo and socket special
files compatible with SFU format to achieve SFU interop, as it is expected
by sfu mount option.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifssmb.c |  8 ++++----
 fs/smb/client/smb1ops.c |  2 +-
 fs/smb/client/smb2ops.c | 29 +++++++++++++++++++----------
 3 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index cfae2e918209..0ffc45aa5e2c 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -1076,8 +1076,8 @@ SMBLegacyOpen(const unsigned int xid, struct cifs_tcon *tcon,
 	pSMB->OpenFlags |= cpu_to_le16(REQ_MORE_INFO);
 	pSMB->Mode = cpu_to_le16(access_flags_to_smbopen_mode(access_flags));
 	pSMB->Mode |= cpu_to_le16(0x40); /* deny none */
-	/* set file as system file if special file such
-	   as fifo and server expecting SFU style and
+	/* set file as system file if special file such as fifo,
+	 * socket, char or block and server expecting SFU style and
 	   no Unix extensions */
 
 	if (create_options & CREATE_OPTION_SPECIAL)
@@ -1193,8 +1193,8 @@ CIFS_open(const unsigned int xid, struct cifs_open_parms *oparms, int *oplock,
 	req->AllocationSize = 0;
 
 	/*
-	 * Set file as system file if special file such as fifo and server
-	 * expecting SFU style and no Unix extensions.
+	 * Set file as system file if special file such as fifo, socket, char
+	 * or block and server expecting SFU style and no Unix extensions.
 	 */
 	if (create_options & CREATE_OPTION_SPECIAL)
 		req->FileAttributes = cpu_to_le32(ATTR_SYSTEM);
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index e1f2feb56f45..e03c91a49650 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -1078,7 +1078,7 @@ cifs_make_node(unsigned int xid, struct inode *inode,
 	/*
 	 * Check if mounted with mount parm 'sfu' mount parm.
 	 * SFU emulation should work with all servers, but only
-	 * supports block and char device (no socket & fifo),
+	 * supports block and char device, socket & fifo,
 	 * and was used by default in earlier versions of Windows
 	 */
 	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL))
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 9c2d065d3cc4..9e90672caf60 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -5066,26 +5066,32 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 	struct cifs_fid fid;
 	unsigned int bytes_written;
 	struct win_dev pdev = {};
+	size_t pdev_len = 0;
 	struct kvec iov[2];
 	__u32 oplock = server->oplocks ? REQ_OPLOCK : 0;
 	int rc;
 
 	switch (mode & S_IFMT) {
 	case S_IFCHR:
+		pdev_len = sizeof(pdev);
 		memcpy(pdev.type, "IntxCHR\0", 8);
 		pdev.major = cpu_to_le64(MAJOR(dev));
 		pdev.minor = cpu_to_le64(MINOR(dev));
 		break;
 	case S_IFBLK:
+		pdev_len = sizeof(pdev);
 		memcpy(pdev.type, "IntxBLK\0", 8);
 		pdev.major = cpu_to_le64(MAJOR(dev));
 		pdev.minor = cpu_to_le64(MINOR(dev));
 		break;
 	case S_IFSOCK:
-		strscpy(pdev.type, "LnxSOCK");
+		/* SFU socket is system file with one zero byte */
+		pdev_len = 1;
+		pdev.type[0] = '\0';
 		break;
 	case S_IFIFO:
-		strscpy(pdev.type, "LnxFIFO");
+		/* SFU fifo is system file which is empty */
+		pdev_len = 0;
 		break;
 	default:
 		return -EPERM;
@@ -5100,14 +5106,17 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 	if (rc)
 		return rc;
 
-	io_parms.pid = current->tgid;
-	io_parms.tcon = tcon;
-	io_parms.length = sizeof(pdev);
-	iov[1].iov_base = &pdev;
-	iov[1].iov_len = sizeof(pdev);
+	if (pdev_len > 0) {
+		io_parms.pid = current->tgid;
+		io_parms.tcon = tcon;
+		io_parms.length = pdev_len;
+		iov[1].iov_base = &pdev;
+		iov[1].iov_len = pdev_len;
+
+		rc = server->ops->sync_write(xid, &fid, &io_parms,
+					     &bytes_written, iov, 1);
+	}
 
-	rc = server->ops->sync_write(xid, &fid, &io_parms,
-				     &bytes_written, iov, 1);
 	server->ops->close(xid, tcon, &fid);
 	return rc;
 }
@@ -5149,7 +5158,7 @@ static int smb2_make_node(unsigned int xid, struct inode *inode,
 	/*
 	 * Check if mounted with mount parm 'sfu' mount parm.
 	 * SFU emulation should work with all servers, but only
-	 * supports block and char device (no socket & fifo),
+	 * supports block and char device, socket & fifo,
 	 * and was used by default in earlier versions of Windows
 	 */
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 7/7] cifs: Add support for creating SFU symlinks
  2024-09-12 12:05 [PATCH 0/7] cifs: Improve client SFU support for special files Pali Rohár
                   ` (5 preceding siblings ...)
  2024-09-12 12:05 ` [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files Pali Rohár
@ 2024-09-12 12:05 ` Pali Rohár
  2024-09-13  3:44 ` [PATCH 0/7] cifs: Improve client SFU support for special files Steve French
  7 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2024-09-12 12:05 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

When 'sfu' mount option is specified and 'mfsymlinks' is not specified then
create new symlinks in SFU-style. This will provide full SFU compatibility
when mounting share with 'sfu' option. 'mfsymlinks' override SFU for better
Apple compatibility as explained in fs_context.c smb3_update_mnt_flags()
function.

Extend __cifs_sfu_make_node() function, which now can handle also S_IFLNK
type and refactor structures passed to sync_write() in this function, by
splitting SFU type ("IntxCHR\0", "IntxBLK\0", "IntxLNK\1", "\0", "") and
SFU data from original combined struct win_dev (which could be used only
for block and char devices which had major and minor parts). This is needed
because type is variable-length and data is type specific.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifspdu.h    |  6 ----
 fs/smb/client/cifsproto.h  |  4 +++
 fs/smb/client/fs_context.c | 13 ++++---
 fs/smb/client/link.c       |  3 ++
 fs/smb/client/smb2ops.c    | 71 +++++++++++++++++++++++++++-----------
 5 files changed, 66 insertions(+), 31 deletions(-)

diff --git a/fs/smb/client/cifspdu.h b/fs/smb/client/cifspdu.h
index a2072ab9e586..c3b6263060b0 100644
--- a/fs/smb/client/cifspdu.h
+++ b/fs/smb/client/cifspdu.h
@@ -2573,12 +2573,6 @@ typedef struct {
 } __attribute__((packed)) FIND_FILE_STANDARD_INFO; /* level 0x1 FF resp data */
 
 
-struct win_dev {
-	unsigned char type[8]; /* IntxCHR or IntxBLK or LnxFIFO or LnxSOCK */
-	__le64 major;
-	__le64 minor;
-} __attribute__((packed));
-
 struct fea {
 	unsigned char EA_flags;
 	__u8 name_len;
diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index 497bf3c447bc..791bddac0396 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -676,6 +676,10 @@ char *extract_sharename(const char *unc);
 int parse_reparse_point(struct reparse_data_buffer *buf,
 			u32 plen, struct cifs_sb_info *cifs_sb,
 			bool unicode, struct cifs_open_info_data *data);
+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,
+			 const char *symname);
 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);
diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index bc926ab2555b..2f0c3894b0f7 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -1896,14 +1896,17 @@ void smb3_update_mnt_flags(struct cifs_sb_info *cifs_sb)
 	if (ctx->mfsymlinks) {
 		if (ctx->sfu_emul) {
 			/*
-			 * Our SFU ("Services for Unix" emulation does not allow
-			 * creating symlinks but does allow reading existing SFU
-			 * symlinks (it does allow both creating and reading SFU
-			 * style mknod and FIFOs though). When "mfsymlinks" and
+			 * Our SFU ("Services for Unix") emulation allows now
+			 * creating new and reading existing SFU symlinks.
+			 * Older Linux kernel versions were not able to neither
+			 * read existing nor create new SFU symlinks. But
+			 * creating and reading SFU style mknod and FIFOs was
+			 * supported for long time. When "mfsymlinks" and
 			 * "sfu" are both enabled at the same time, it allows
 			 * reading both types of symlinks, but will only create
 			 * them with mfsymlinks format. This allows better
-			 * Apple compatibility (probably better for Samba too)
+			 * Apple compatibility, compatibility with older Linux
+			 * kernel clients (probably better for Samba too)
 			 * while still recognizing old Windows style symlinks.
 			 */
 			cifs_dbg(VFS, "mount options mfsymlinks and sfu both enabled\n");
diff --git a/fs/smb/client/link.c b/fs/smb/client/link.c
index 80099bbb333b..47ddeb7fa111 100644
--- a/fs/smb/client/link.c
+++ b/fs/smb/client/link.c
@@ -606,6 +606,9 @@ cifs_symlink(struct mnt_idmap *idmap, struct inode *inode,
 	/* BB what if DFS and this volume is on different share? BB */
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) {
 		rc = create_mf_symlink(xid, pTcon, cifs_sb, full_path, symname);
+	} else if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) {
+		rc = __cifs_sfu_make_node(xid, inode, direntry, pTcon,
+					  full_path, S_IFLNK, 0, symname);
 #ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
 	} else if (pTcon->unix_ext) {
 		rc = CIFSUnixCreateSymLink(xid, pTcon, full_path, symname,
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 9e90672caf60..b77a62ccec5c 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -5055,9 +5055,10 @@ static int smb2_next_header(struct TCP_Server_Info *server, char *buf,
 	return 0;
 }
 
-static 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)
+				const char *full_path, umode_t mode, dev_t dev,
+				const char *symname)
 {
 	struct TCP_Server_Info *server = tcon->ses->server;
 	struct cifs_open_parms oparms;
@@ -5065,36 +5066,61 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
 	struct cifs_fid fid;
 	unsigned int bytes_written;
-	struct win_dev pdev = {};
-	size_t pdev_len = 0;
-	struct kvec iov[2];
+	u8 type[8];
+	int type_len = 0;
+	struct {
+		__le64 major;
+		__le64 minor;
+	} __packed pdev = {};
+	__le16 *symname_utf16 = NULL;
+	u8 *data = NULL;
+	int data_len = 0;
+	struct kvec iov[3];
 	__u32 oplock = server->oplocks ? REQ_OPLOCK : 0;
 	int rc;
 
 	switch (mode & S_IFMT) {
 	case S_IFCHR:
-		pdev_len = sizeof(pdev);
-		memcpy(pdev.type, "IntxCHR\0", 8);
+		type_len = 8;
+		memcpy(type, "IntxCHR\0", type_len);
 		pdev.major = cpu_to_le64(MAJOR(dev));
 		pdev.minor = cpu_to_le64(MINOR(dev));
+		data = (u8 *)&pdev;
+		data_len = sizeof(pdev);
 		break;
 	case S_IFBLK:
-		pdev_len = sizeof(pdev);
-		memcpy(pdev.type, "IntxBLK\0", 8);
+		type_len = 8;
+		memcpy(type, "IntxBLK\0", type_len);
 		pdev.major = cpu_to_le64(MAJOR(dev));
 		pdev.minor = cpu_to_le64(MINOR(dev));
+		data = (u8 *)&pdev;
+		data_len = sizeof(pdev);
+		break;
+	case S_IFLNK:
+		type_len = 8;
+		memcpy(type, "IntxLNK\1", type_len);
+		symname_utf16 = cifs_strndup_to_utf16(symname, strlen(symname),
+						      &data_len, cifs_sb->local_nls,
+						      NO_MAP_UNI_RSVD);
+		if (!symname_utf16) {
+			rc = -ENOMEM;
+			goto out;
+		}
+		data_len -= 2; /* symlink is without trailing wide-nul */
+		data = (u8 *)symname_utf16;
 		break;
 	case S_IFSOCK:
 		/* SFU socket is system file with one zero byte */
-		pdev_len = 1;
-		pdev.type[0] = '\0';
+		type_len = 1;
+		type[0] = '\0';
 		break;
 	case S_IFIFO:
 		/* SFU fifo is system file which is empty */
-		pdev_len = 0;
+		type_len = 0;
 		break;
 	default:
-		return -EPERM;
+		rc = -EPERM;
+		goto out;
 	}
 
 	oparms = CIFS_OPARMS(cifs_sb, tcon, full_path, GENERIC_WRITE,
@@ -5104,20 +5130,25 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 
 	rc = server->ops->open(xid, &oparms, &oplock, NULL);
 	if (rc)
-		return rc;
+		goto out;
 
-	if (pdev_len > 0) {
+	if (type_len + data_len > 0) {
 		io_parms.pid = current->tgid;
 		io_parms.tcon = tcon;
-		io_parms.length = pdev_len;
-		iov[1].iov_base = &pdev;
-		iov[1].iov_len = pdev_len;
+		io_parms.length = type_len + data_len;
+		iov[1].iov_base = type;
+		iov[1].iov_len = type_len;
+		iov[2].iov_base = data;
+		iov[2].iov_len = data_len;
 
 		rc = server->ops->sync_write(xid, &fid, &io_parms,
-					     &bytes_written, iov, 1);
+					     &bytes_written, iov, 2);
 	}
 
 	server->ops->close(xid, tcon, &fid);
+
+out:
+	kfree(symname_utf16);
 	return rc;
 }
 
@@ -5129,7 +5160,7 @@ int cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 	int rc;
 
 	rc = __cifs_sfu_make_node(xid, inode, dentry, tcon,
-				  full_path, mode, dev);
+				  full_path, mode, dev, NULL);
 	if (rc)
 		return rc;
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/7] cifs: Improve client SFU support for special files
  2024-09-12 12:05 [PATCH 0/7] cifs: Improve client SFU support for special files Pali Rohár
                   ` (6 preceding siblings ...)
  2024-09-12 12:05 ` [PATCH 7/7] cifs: Add support for creating SFU symlinks Pali Rohár
@ 2024-09-13  3:44 ` Steve French
  2024-09-13  7:42   ` Pali Rohár
  7 siblings, 1 reply; 30+ messages in thread
From: Steve French @ 2024-09-13  3:44 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, linux-cifs,
	linux-kernel

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

Do you remember if there is a reference to  sections of the protocol
documentation that can help confirm some of the changes?

On Thu, Sep 12, 2024 at 7:06 AM Pali Rohár <pali@kernel.org> wrote:
>
> This patch series implement full support for SFU-style of special files
> (fifo, socket, block, char, symlink) and makes client cifs's -o sfu
> mount option to be fully compatible with SFU.
>
> Before this patch series, kernel cifs client was able to:
> * create new char and block devices in SFU-style, and detect them
> * detect existing SFU fifo, but no create/mkfifo SFU functionality
> * detect symlink SFU symlink, but no readlink() functionality and
>   neither no create SFU symlink functionality
>
> And it was able to create some SFU-incompatible sockets and fifos (when
> -o sfu was specified) which were not recognized by neither original MS
> SFU implementation and neither by others.
>
> This patch series implements missing functionality, which is:
> * detect existing SFU sockets
> * create new SFU sockets
> * create new SFU fifos
> * create new SFU symlink
> * readlink() support for existing SFU symlinks
>
> In following pragraphs are some details about these SFU special files
> which usage on Linux has to be activated by -o sfu mount option.
>
> SFU-style fifo is empty file with system attribute set. This format is
> used by old Microsoft POSIX subsystem and later also by OpenNT/Interix
> subsystem (which replaced Microsoft POSIX subsystem and is part of
> Microsoft SFU). OpenNT is previous name of Interix versions 1.x/2.x.
> Microsoft POSIX subsystem is available since the first Windows NT
> version 3.1, and it was replaced by Interix since Windows XP. Interix
> continue to use this format up to the its last released version for
> Windows 8 / Server 2012 (part of Subsystem for UNIX-based Applications).
> Hence these SFU-style fifos are for very long time unified and widely
> supported.
>
> SFU-style socket is file which has system attribute set and file content
> is one zero byte.
>
> SFU-style symlink is file which has system attribute set and file content
> is buffer "IntxLNK\1" (8th byte is 0x01) followed by the target location
> encoded in little endian UCS-2/UTF-16. There is no trailing nul.
>
> SFU-style char or block device is file which has system attribute set
> and file content is buffer "IntxBLK\0" or "IntxCHR\0" (8th byte is 0x00)
> followed by major and minor numbers encoded in twos 64-bit little endian.
>
> Format of SFU-style sockets, symlinks, char and block devices was
> introduced in Interix 3.0 subsystem, as part of the Microsoft SFU 3.0
> and is used also by all later versions, up to the Windows 8 / Server 2012
> (part of Subsystem for UNIX-based Applications) where it was deprecated.
> Previous OpenNT/Interix versions had no support for UNIX domain sockets
> (and socket files), symlinks or possibility to create character or block
> devices (but they had block/char devices in-memory, returned by stat()).
>
> Microsoft NFS server up to the version included in Windows Server 2008 R2
> also uses this SFU-style format of special files when storing them on
> local filesystem. Later Microsoft NFS server versions (starting in
> Windows Server 2012) use new NFS reparse format, which Interix subsystem
> (included in SFU or SUA) does not understand.
>
> Even SFU-style of special files is old format, it has one big advantage,
> this format does not require any support on SMB/CIFS server of special
> files, as everything is stored only in the file content. The only
> requirement from server is support for system attribute. So this allows
> to store special files also on FAT filesystem.
>
> This patch series makes cifs -o sfu mount option compatible with
> SFU-style of special files, and so compatible with the latest SFU/SUA.
>
> Note that -o sfu is by default turned off, so these changes should have
> no effect on default cifs mounts.
>
> Manually tested with MS SFU 3.5 (for Windows XP) and MS SUA 6.2 (latest
> released version of Interix) that interop works correctly, special files
> created by POSIX/Interix application can be recognized by Linux cifs
> client (exported over MS SMB) with these patches (and vice-versa setup,
> created by Linux cifs client and recognized in POSIX/Interix subsystem).
>
> Manually tested that old Linux 4.19 cifs client version can recognize
> SFU-style of special files created by Linux cifs client this patch
> series (except socket, which is unsupported in this Linux cifs version).
>
> Patch series is based on the latest upstream tag v6.11-rc7.
>
> Pali Rohár (7):
>   cifs: Fix recognizing SFU symlinks
>   cifs: Add support for reading SFU symlink location
>   cifs: Put explicit zero byte into SFU block/char types
>   cifs: Show debug message when SFU Fifo type was detected
>   cifs: Recognize SFU socket type
>   cifs: Fix creating of SFU fifo and socket special files
>   cifs: Add support for creating SFU symlinks
>
>  fs/smb/client/cifspdu.h    |  6 ---
>  fs/smb/client/cifsproto.h  |  4 ++
>  fs/smb/client/cifssmb.c    |  8 ++--
>  fs/smb/client/fs_context.c | 13 ++++---
>  fs/smb/client/inode.c      | 42 ++++++++++++++++++--
>  fs/smb/client/link.c       |  3 ++
>  fs/smb/client/smb1ops.c    |  2 +-
>  fs/smb/client/smb2ops.c    | 78 ++++++++++++++++++++++++++++----------
>  8 files changed, 117 insertions(+), 39 deletions(-)
>
> --
> 2.20.1
>
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/7] cifs: Improve client SFU support for special files
  2024-09-13  3:44 ` [PATCH 0/7] cifs: Improve client SFU support for special files Steve French
@ 2024-09-13  7:42   ` Pali Rohár
  0 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2024-09-13  7:42 UTC (permalink / raw)
  To: Steve French
  Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, linux-cifs,
	linux-kernel

On Thursday 12 September 2024 22:44:40 Steve French wrote:
> tentatively merged into cifs-2.6.git for-next pending review and more testing
> 
> Do you remember if there is a reference to  sections of the protocol
> documentation that can help confirm some of the changes?

I do not know about normative documentation of SFU-style special files.
If there is some documentation about it then it would be part of either
Interix subsystem or part of NTFS. And I'm not sure if either of those
were released to public. All things were done based on observations.

> On Thu, Sep 12, 2024 at 7:06 AM Pali Rohár <pali@kernel.org> wrote:
> >
> > This patch series implement full support for SFU-style of special files
> > (fifo, socket, block, char, symlink) and makes client cifs's -o sfu
> > mount option to be fully compatible with SFU.
> >
> > Before this patch series, kernel cifs client was able to:
> > * create new char and block devices in SFU-style, and detect them
> > * detect existing SFU fifo, but no create/mkfifo SFU functionality
> > * detect symlink SFU symlink, but no readlink() functionality and
> >   neither no create SFU symlink functionality
> >
> > And it was able to create some SFU-incompatible sockets and fifos (when
> > -o sfu was specified) which were not recognized by neither original MS
> > SFU implementation and neither by others.
> >
> > This patch series implements missing functionality, which is:
> > * detect existing SFU sockets
> > * create new SFU sockets
> > * create new SFU fifos
> > * create new SFU symlink
> > * readlink() support for existing SFU symlinks
> >
> > In following pragraphs are some details about these SFU special files
> > which usage on Linux has to be activated by -o sfu mount option.
> >
> > SFU-style fifo is empty file with system attribute set. This format is
> > used by old Microsoft POSIX subsystem and later also by OpenNT/Interix
> > subsystem (which replaced Microsoft POSIX subsystem and is part of
> > Microsoft SFU). OpenNT is previous name of Interix versions 1.x/2.x.
> > Microsoft POSIX subsystem is available since the first Windows NT
> > version 3.1, and it was replaced by Interix since Windows XP. Interix
> > continue to use this format up to the its last released version for
> > Windows 8 / Server 2012 (part of Subsystem for UNIX-based Applications).
> > Hence these SFU-style fifos are for very long time unified and widely
> > supported.
> >
> > SFU-style socket is file which has system attribute set and file content
> > is one zero byte.
> >
> > SFU-style symlink is file which has system attribute set and file content
> > is buffer "IntxLNK\1" (8th byte is 0x01) followed by the target location
> > encoded in little endian UCS-2/UTF-16. There is no trailing nul.
> >
> > SFU-style char or block device is file which has system attribute set
> > and file content is buffer "IntxBLK\0" or "IntxCHR\0" (8th byte is 0x00)
> > followed by major and minor numbers encoded in twos 64-bit little endian.
> >
> > Format of SFU-style sockets, symlinks, char and block devices was
> > introduced in Interix 3.0 subsystem, as part of the Microsoft SFU 3.0
> > and is used also by all later versions, up to the Windows 8 / Server 2012
> > (part of Subsystem for UNIX-based Applications) where it was deprecated.
> > Previous OpenNT/Interix versions had no support for UNIX domain sockets
> > (and socket files), symlinks or possibility to create character or block
> > devices (but they had block/char devices in-memory, returned by stat()).
> >
> > Microsoft NFS server up to the version included in Windows Server 2008 R2
> > also uses this SFU-style format of special files when storing them on
> > local filesystem. Later Microsoft NFS server versions (starting in
> > Windows Server 2012) use new NFS reparse format, which Interix subsystem
> > (included in SFU or SUA) does not understand.
> >
> > Even SFU-style of special files is old format, it has one big advantage,
> > this format does not require any support on SMB/CIFS server of special
> > files, as everything is stored only in the file content. The only
> > requirement from server is support for system attribute. So this allows
> > to store special files also on FAT filesystem.
> >
> > This patch series makes cifs -o sfu mount option compatible with
> > SFU-style of special files, and so compatible with the latest SFU/SUA.
> >
> > Note that -o sfu is by default turned off, so these changes should have
> > no effect on default cifs mounts.
> >
> > Manually tested with MS SFU 3.5 (for Windows XP) and MS SUA 6.2 (latest
> > released version of Interix) that interop works correctly, special files
> > created by POSIX/Interix application can be recognized by Linux cifs
> > client (exported over MS SMB) with these patches (and vice-versa setup,
> > created by Linux cifs client and recognized in POSIX/Interix subsystem).
> >
> > Manually tested that old Linux 4.19 cifs client version can recognize
> > SFU-style of special files created by Linux cifs client this patch
> > series (except socket, which is unsupported in this Linux cifs version).
> >
> > Patch series is based on the latest upstream tag v6.11-rc7.
> >
> > Pali Rohár (7):
> >   cifs: Fix recognizing SFU symlinks
> >   cifs: Add support for reading SFU symlink location
> >   cifs: Put explicit zero byte into SFU block/char types
> >   cifs: Show debug message when SFU Fifo type was detected
> >   cifs: Recognize SFU socket type
> >   cifs: Fix creating of SFU fifo and socket special files
> >   cifs: Add support for creating SFU symlinks
> >
> >  fs/smb/client/cifspdu.h    |  6 ---
> >  fs/smb/client/cifsproto.h  |  4 ++
> >  fs/smb/client/cifssmb.c    |  8 ++--
> >  fs/smb/client/fs_context.c | 13 ++++---
> >  fs/smb/client/inode.c      | 42 ++++++++++++++++++--
> >  fs/smb/client/link.c       |  3 ++
> >  fs/smb/client/smb1ops.c    |  2 +-
> >  fs/smb/client/smb2ops.c    | 78 ++++++++++++++++++++++++++++----------
> >  8 files changed, 117 insertions(+), 39 deletions(-)
> >
> > --
> > 2.20.1
> >
> >
> 
> 
> -- 
> Thanks,
> 
> Steve

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/7] cifs: Fix recognizing SFU symlinks
  2024-09-12 12:05 ` [PATCH 1/7] cifs: Fix recognizing SFU symlinks Pali Rohár
@ 2024-09-13 20:04   ` Pali Rohár
  0 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2024-09-13 20:04 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

On Thursday 12 September 2024 14:05:42 Pali Rohár wrote:
> SFU symlinks have 8 byte prefix: "IntxLNK\1".
> So check also the last 8th byte 0x01.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Fixes: 9e294f1c4d4a ("[CIFS] Recognize properly symlinks and char/blk devices (not just FIFOs) created by SFU (part 2 of 2).")

(I located commit which probably by mistake removed that byte 0x01).

> ---
>  fs/smb/client/inode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> index 73e2e6c230b7..7d424e769a56 100644
> --- a/fs/smb/client/inode.c
> +++ b/fs/smb/client/inode.c
> @@ -612,7 +612,7 @@ cifs_sfu_type(struct cifs_fattr *fattr, const char *path,
>  			cifs_dbg(FYI, "Socket\n");
>  			fattr->cf_mode |= S_IFSOCK;
>  			fattr->cf_dtype = DT_SOCK;
> -		} else if (memcmp("IntxLNK", pbuf, 7) == 0) {
> +		} else if (memcmp("IntxLNK\1", pbuf, 8) == 0) {
>  			cifs_dbg(FYI, "Symlink\n");
>  			fattr->cf_mode |= S_IFLNK;
>  			fattr->cf_dtype = DT_LNK;
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files
  2024-09-12 12:05 ` [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files Pali Rohár
@ 2024-09-13 20:07   ` Pali Rohár
  2024-09-13 22:14     ` Steve French
  2024-09-15 19:45   ` [PATCH 0/4] cifs: Improve client SFU support for special files (2) Pali Rohár
  1 sibling, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2024-09-13 20:07 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

On Thursday 12 September 2024 14:05:47 Pali Rohár wrote:
> SFU-style fifo is empty file with system attribute set. This format is used
> by old Microsoft POSIX subsystem and later also by OpenNT/Interix subsystem
> (which replaced Microsoft POSIX subsystem and is part of Microsoft SFU).
> 
> SFU-style socket is file which has system attribute set and file content is
> one zero byte. This format was introduced in Interix 3.0 subsystem, as part
> of the Microsoft SFU 3.0 and is used also by all later versions. Previous
> versions had no UNIX domain socket support.
> 
> Currently when sfu mount option is specified then CIFS creates fifo and
> socket special files with some strange LnxSOCK or LnxFIFO content which is
> not compatible with Microsoft SFU and neither recognized by other SMB
> implementations which have some SFU support, including older Linux cifs
> implementations.
> 
> So when sfu mount option is specified, create all fifo and socket special
> files compatible with SFU format to achieve SFU interop, as it is expected
> by sfu mount option.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Fixes: 72bc63f5e23a ("smb3: fix creating FIFOs when mounting with "sfu" mount option")
Fixes: 518549c120e6 ("cifs: fix creating sockets when using sfu mount options")

I located commits which introduced those strange LnxSOCK or LnxFIFO
types which are not compatible with SFU. I would suggest to add those
two Fixes: tags into commit message for reference.

> ---
>  fs/smb/client/cifssmb.c |  8 ++++----
>  fs/smb/client/smb1ops.c |  2 +-
>  fs/smb/client/smb2ops.c | 29 +++++++++++++++++++----------
>  3 files changed, 24 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
> index cfae2e918209..0ffc45aa5e2c 100644
> --- a/fs/smb/client/cifssmb.c
> +++ b/fs/smb/client/cifssmb.c
> @@ -1076,8 +1076,8 @@ SMBLegacyOpen(const unsigned int xid, struct cifs_tcon *tcon,
>  	pSMB->OpenFlags |= cpu_to_le16(REQ_MORE_INFO);
>  	pSMB->Mode = cpu_to_le16(access_flags_to_smbopen_mode(access_flags));
>  	pSMB->Mode |= cpu_to_le16(0x40); /* deny none */
> -	/* set file as system file if special file such
> -	   as fifo and server expecting SFU style and
> +	/* set file as system file if special file such as fifo,
> +	 * socket, char or block and server expecting SFU style and
>  	   no Unix extensions */
>  
>  	if (create_options & CREATE_OPTION_SPECIAL)
> @@ -1193,8 +1193,8 @@ CIFS_open(const unsigned int xid, struct cifs_open_parms *oparms, int *oplock,
>  	req->AllocationSize = 0;
>  
>  	/*
> -	 * Set file as system file if special file such as fifo and server
> -	 * expecting SFU style and no Unix extensions.
> +	 * Set file as system file if special file such as fifo, socket, char
> +	 * or block and server expecting SFU style and no Unix extensions.
>  	 */
>  	if (create_options & CREATE_OPTION_SPECIAL)
>  		req->FileAttributes = cpu_to_le32(ATTR_SYSTEM);
> diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
> index e1f2feb56f45..e03c91a49650 100644
> --- a/fs/smb/client/smb1ops.c
> +++ b/fs/smb/client/smb1ops.c
> @@ -1078,7 +1078,7 @@ cifs_make_node(unsigned int xid, struct inode *inode,
>  	/*
>  	 * Check if mounted with mount parm 'sfu' mount parm.
>  	 * SFU emulation should work with all servers, but only
> -	 * supports block and char device (no socket & fifo),
> +	 * supports block and char device, socket & fifo,
>  	 * and was used by default in earlier versions of Windows
>  	 */
>  	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL))
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> index 9c2d065d3cc4..9e90672caf60 100644
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -5066,26 +5066,32 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
>  	struct cifs_fid fid;
>  	unsigned int bytes_written;
>  	struct win_dev pdev = {};
> +	size_t pdev_len = 0;
>  	struct kvec iov[2];
>  	__u32 oplock = server->oplocks ? REQ_OPLOCK : 0;
>  	int rc;
>  
>  	switch (mode & S_IFMT) {
>  	case S_IFCHR:
> +		pdev_len = sizeof(pdev);
>  		memcpy(pdev.type, "IntxCHR\0", 8);
>  		pdev.major = cpu_to_le64(MAJOR(dev));
>  		pdev.minor = cpu_to_le64(MINOR(dev));
>  		break;
>  	case S_IFBLK:
> +		pdev_len = sizeof(pdev);
>  		memcpy(pdev.type, "IntxBLK\0", 8);
>  		pdev.major = cpu_to_le64(MAJOR(dev));
>  		pdev.minor = cpu_to_le64(MINOR(dev));
>  		break;
>  	case S_IFSOCK:
> -		strscpy(pdev.type, "LnxSOCK");
> +		/* SFU socket is system file with one zero byte */
> +		pdev_len = 1;
> +		pdev.type[0] = '\0';
>  		break;
>  	case S_IFIFO:
> -		strscpy(pdev.type, "LnxFIFO");
> +		/* SFU fifo is system file which is empty */
> +		pdev_len = 0;
>  		break;
>  	default:
>  		return -EPERM;
> @@ -5100,14 +5106,17 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
>  	if (rc)
>  		return rc;
>  
> -	io_parms.pid = current->tgid;
> -	io_parms.tcon = tcon;
> -	io_parms.length = sizeof(pdev);
> -	iov[1].iov_base = &pdev;
> -	iov[1].iov_len = sizeof(pdev);
> +	if (pdev_len > 0) {
> +		io_parms.pid = current->tgid;
> +		io_parms.tcon = tcon;
> +		io_parms.length = pdev_len;
> +		iov[1].iov_base = &pdev;
> +		iov[1].iov_len = pdev_len;
> +
> +		rc = server->ops->sync_write(xid, &fid, &io_parms,
> +					     &bytes_written, iov, 1);
> +	}
>  
> -	rc = server->ops->sync_write(xid, &fid, &io_parms,
> -				     &bytes_written, iov, 1);
>  	server->ops->close(xid, tcon, &fid);
>  	return rc;
>  }
> @@ -5149,7 +5158,7 @@ static int smb2_make_node(unsigned int xid, struct inode *inode,
>  	/*
>  	 * Check if mounted with mount parm 'sfu' mount parm.
>  	 * SFU emulation should work with all servers, but only
> -	 * supports block and char device (no socket & fifo),
> +	 * supports block and char device, socket & fifo,
>  	 * and was used by default in earlier versions of Windows
>  	 */
>  	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) {
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files
  2024-09-13 20:07   ` Pali Rohár
@ 2024-09-13 22:14     ` Steve French
  2024-09-13 22:33       ` Steve French
  2024-09-13 22:42       ` Pali Rohár
  0 siblings, 2 replies; 30+ messages in thread
From: Steve French @ 2024-09-13 22:14 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, linux-cifs,
	linux-kernel

How did you find the format of the FIFO and SOCK file types?  I
couldn't find any references to those so added two new types to allow
current Linux to be able to create these (especially since they are
opaque to the server and thus low risk).

> +     case S_IFSOCK:
> -             strscpy(pdev.type, "LnxSOCK");
> +             /* SFU socket is system file with one zero byte */
> +             pdev_len = 1;
> +             pdev.type[0] = '\0';
>               break;
>       case S_IFIFO:
> -             strscpy(pdev.type, "LnxFIFO");
> +             /* SFU fifo is system file which is empty */
> +             pdev_len = 0;

My worry about the suggested change above is that it is possible that
we could accidentally match to an empty file. We intentionally added
the two new dev.type fields for these to avoid collisions with other
things (and since they are Linux specific).  It seems risky to have an
empty file with the system attribute marked as a FIFO, and similarly a
file with one byte null as Socket.   Since this is for only the Linux
client to recognize, I wanted to do something safe for those file
types less likely to be confusing (ie strings for Socket and FIFO that
were similar in length and format to the other special files seemed
intuitive) and "LnxFIFO" and LnxSOCK" were used as the tags in the
file to reduce confusion ie the tags for those two start with "Lnx" -
ie "something used for Linux client" not related to the original
Interix (those begin with "Intx").

Note that in the long run we hope to use reparse points by default in
more servers to store special files like this but there are a few
cases for unusual workloads that need special file support that would
have to use sfu still.  The newer reparse tags that Windows uses "WSL"
have the advantage that they require fewer roundtrips to query (since
the file type is in the reparse tag).

Also noticed an interesting problem when mounted with "sfu" -
"smbgetinfo filebasicinfo /mnt/fifo1" will hang (in sys_open).  Is
that expected for a FIFO?

On Fri, Sep 13, 2024 at 3:07 PM Pali Rohár <pali@kernel.org> wrote:
>
> On Thursday 12 September 2024 14:05:47 Pali Rohár wrote:
> > SFU-style fifo is empty file with system attribute set. This format is used
> > by old Microsoft POSIX subsystem and later also by OpenNT/Interix subsystem
> > (which replaced Microsoft POSIX subsystem and is part of Microsoft SFU).
> >
> > SFU-style socket is file which has system attribute set and file content is
> > one zero byte. This format was introduced in Interix 3.0 subsystem, as part
> > of the Microsoft SFU 3.0 and is used also by all later versions. Previous
> > versions had no UNIX domain socket support.
> >
> > Currently when sfu mount option is specified then CIFS creates fifo and
> > socket special files with some strange LnxSOCK or LnxFIFO content which is
> > not compatible with Microsoft SFU and neither recognized by other SMB
> > implementations which have some SFU support, including older Linux cifs
> > implementations.
> >
> > So when sfu mount option is specified, create all fifo and socket special
> > files compatible with SFU format to achieve SFU interop, as it is expected
> > by sfu mount option.
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
>
> Fixes: 72bc63f5e23a ("smb3: fix creating FIFOs when mounting with "sfu" mount option")
> Fixes: 518549c120e6 ("cifs: fix creating sockets when using sfu mount options")
>
> I located commits which introduced those strange LnxSOCK or LnxFIFO
> types which are not compatible with SFU. I would suggest to add those
> two Fixes: tags into commit message for reference.
>
> > ---
> >  fs/smb/client/cifssmb.c |  8 ++++----
> >  fs/smb/client/smb1ops.c |  2 +-
> >  fs/smb/client/smb2ops.c | 29 +++++++++++++++++++----------
> >  3 files changed, 24 insertions(+), 15 deletions(-)
> >
> > diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
> > index cfae2e918209..0ffc45aa5e2c 100644
> > --- a/fs/smb/client/cifssmb.c
> > +++ b/fs/smb/client/cifssmb.c
> > @@ -1076,8 +1076,8 @@ SMBLegacyOpen(const unsigned int xid, struct cifs_tcon *tcon,
> >       pSMB->OpenFlags |= cpu_to_le16(REQ_MORE_INFO);
> >       pSMB->Mode = cpu_to_le16(access_flags_to_smbopen_mode(access_flags));
> >       pSMB->Mode |= cpu_to_le16(0x40); /* deny none */
> > -     /* set file as system file if special file such
> > -        as fifo and server expecting SFU style and
> > +     /* set file as system file if special file such as fifo,
> > +      * socket, char or block and server expecting SFU style and
> >          no Unix extensions */
> >
> >       if (create_options & CREATE_OPTION_SPECIAL)
> > @@ -1193,8 +1193,8 @@ CIFS_open(const unsigned int xid, struct cifs_open_parms *oparms, int *oplock,
> >       req->AllocationSize = 0;
> >
> >       /*
> > -      * Set file as system file if special file such as fifo and server
> > -      * expecting SFU style and no Unix extensions.
> > +      * Set file as system file if special file such as fifo, socket, char
> > +      * or block and server expecting SFU style and no Unix extensions.
> >        */
> >       if (create_options & CREATE_OPTION_SPECIAL)
> >               req->FileAttributes = cpu_to_le32(ATTR_SYSTEM);
> > diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
> > index e1f2feb56f45..e03c91a49650 100644
> > --- a/fs/smb/client/smb1ops.c
> > +++ b/fs/smb/client/smb1ops.c
> > @@ -1078,7 +1078,7 @@ cifs_make_node(unsigned int xid, struct inode *inode,
> >       /*
> >        * Check if mounted with mount parm 'sfu' mount parm.
> >        * SFU emulation should work with all servers, but only
> > -      * supports block and char device (no socket & fifo),
> > +      * supports block and char device, socket & fifo,
> >        * and was used by default in earlier versions of Windows
> >        */
> >       if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL))
> > diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> > index 9c2d065d3cc4..9e90672caf60 100644
> > --- a/fs/smb/client/smb2ops.c
> > +++ b/fs/smb/client/smb2ops.c
> > @@ -5066,26 +5066,32 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
> >       struct cifs_fid fid;
> >       unsigned int bytes_written;
> >       struct win_dev pdev = {};
> > +     size_t pdev_len = 0;
> >       struct kvec iov[2];
> >       __u32 oplock = server->oplocks ? REQ_OPLOCK : 0;
> >       int rc;
> >
> >       switch (mode & S_IFMT) {
> >       case S_IFCHR:
> > +             pdev_len = sizeof(pdev);
> >               memcpy(pdev.type, "IntxCHR\0", 8);
> >               pdev.major = cpu_to_le64(MAJOR(dev));
> >               pdev.minor = cpu_to_le64(MINOR(dev));
> >               break;
> >       case S_IFBLK:
> > +             pdev_len = sizeof(pdev);
> >               memcpy(pdev.type, "IntxBLK\0", 8);
> >               pdev.major = cpu_to_le64(MAJOR(dev));
> >               pdev.minor = cpu_to_le64(MINOR(dev));
> >               break;
> >       case S_IFSOCK:
> > -             strscpy(pdev.type, "LnxSOCK");
> > +             /* SFU socket is system file with one zero byte */
> > +             pdev_len = 1;
> > +             pdev.type[0] = '\0';
> >               break;
> >       case S_IFIFO:
> > -             strscpy(pdev.type, "LnxFIFO");
> > +             /* SFU fifo is system file which is empty */
> > +             pdev_len = 0;
> >               break;
> >       default:
> >               return -EPERM;
> > @@ -5100,14 +5106,17 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
> >       if (rc)
> >               return rc;
> >
> > -     io_parms.pid = current->tgid;
> > -     io_parms.tcon = tcon;
> > -     io_parms.length = sizeof(pdev);
> > -     iov[1].iov_base = &pdev;
> > -     iov[1].iov_len = sizeof(pdev);
> > +     if (pdev_len > 0) {
> > +             io_parms.pid = current->tgid;
> > +             io_parms.tcon = tcon;
> > +             io_parms.length = pdev_len;
> > +             iov[1].iov_base = &pdev;
> > +             iov[1].iov_len = pdev_len;
> > +
> > +             rc = server->ops->sync_write(xid, &fid, &io_parms,
> > +                                          &bytes_written, iov, 1);
> > +     }
> >
> > -     rc = server->ops->sync_write(xid, &fid, &io_parms,
> > -                                  &bytes_written, iov, 1);
> >       server->ops->close(xid, tcon, &fid);
> >       return rc;
> >  }
> > @@ -5149,7 +5158,7 @@ static int smb2_make_node(unsigned int xid, struct inode *inode,
> >       /*
> >        * Check if mounted with mount parm 'sfu' mount parm.
> >        * SFU emulation should work with all servers, but only
> > -      * supports block and char device (no socket & fifo),
> > +      * supports block and char device, socket & fifo,
> >        * and was used by default in earlier versions of Windows
> >        */
> >       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) {
> > --
> > 2.20.1
> >
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files
  2024-09-13 22:14     ` Steve French
@ 2024-09-13 22:33       ` Steve French
  2024-09-13 22:45         ` Pali Rohár
  2024-09-13 22:42       ` Pali Rohár
  1 sibling, 1 reply; 30+ messages in thread
From: Steve French @ 2024-09-13 22:33 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, linux-cifs,
	linux-kernel

One other thing which worried me a bit is that I confirmed that an
empty file with the system attribute set was matched as a FIFO file
type (when you mount with "sfu") - this seems risky.

On Fri, Sep 13, 2024 at 5:14 PM Steve French <smfrench@gmail.com> wrote:
>
> How did you find the format of the FIFO and SOCK file types?  I
> couldn't find any references to those so added two new types to allow
> current Linux to be able to create these (especially since they are
> opaque to the server and thus low risk).
>
> > +     case S_IFSOCK:
> > -             strscpy(pdev.type, "LnxSOCK");
> > +             /* SFU socket is system file with one zero byte */
> > +             pdev_len = 1;
> > +             pdev.type[0] = '\0';
> >               break;
> >       case S_IFIFO:
> > -             strscpy(pdev.type, "LnxFIFO");
> > +             /* SFU fifo is system file which is empty */
> > +             pdev_len = 0;
>
> My worry about the suggested change above is that it is possible that
> we could accidentally match to an empty file. We intentionally added
> the two new dev.type fields for these to avoid collisions with other
> things (and since they are Linux specific).  It seems risky to have an
> empty file with the system attribute marked as a FIFO, and similarly a
> file with one byte null as Socket.   Since this is for only the Linux
> client to recognize, I wanted to do something safe for those file
> types less likely to be confusing (ie strings for Socket and FIFO that
> were similar in length and format to the other special files seemed
> intuitive) and "LnxFIFO" and LnxSOCK" were used as the tags in the
> file to reduce confusion ie the tags for those two start with "Lnx" -
> ie "something used for Linux client" not related to the original
> Interix (those begin with "Intx").
>
> Note that in the long run we hope to use reparse points by default in
> more servers to store special files like this but there are a few
> cases for unusual workloads that need special file support that would
> have to use sfu still.  The newer reparse tags that Windows uses "WSL"
> have the advantage that they require fewer roundtrips to query (since
> the file type is in the reparse tag).
>
> Also noticed an interesting problem when mounted with "sfu" -
> "smbgetinfo filebasicinfo /mnt/fifo1" will hang (in sys_open).  Is
> that expected for a FIFO?
>
> On Fri, Sep 13, 2024 at 3:07 PM Pali Rohár <pali@kernel.org> wrote:
> >
> > On Thursday 12 September 2024 14:05:47 Pali Rohár wrote:
> > > SFU-style fifo is empty file with system attribute set. This format is used
> > > by old Microsoft POSIX subsystem and later also by OpenNT/Interix subsystem
> > > (which replaced Microsoft POSIX subsystem and is part of Microsoft SFU).
> > >
> > > SFU-style socket is file which has system attribute set and file content is
> > > one zero byte. This format was introduced in Interix 3.0 subsystem, as part
> > > of the Microsoft SFU 3.0 and is used also by all later versions. Previous
> > > versions had no UNIX domain socket support.
> > >
> > > Currently when sfu mount option is specified then CIFS creates fifo and
> > > socket special files with some strange LnxSOCK or LnxFIFO content which is
> > > not compatible with Microsoft SFU and neither recognized by other SMB
> > > implementations which have some SFU support, including older Linux cifs
> > > implementations.
> > >
> > > So when sfu mount option is specified, create all fifo and socket special
> > > files compatible with SFU format to achieve SFU interop, as it is expected
> > > by sfu mount option.
> > >
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> >
> > Fixes: 72bc63f5e23a ("smb3: fix creating FIFOs when mounting with "sfu" mount option")
> > Fixes: 518549c120e6 ("cifs: fix creating sockets when using sfu mount options")
> >
> > I located commits which introduced those strange LnxSOCK or LnxFIFO
> > types which are not compatible with SFU. I would suggest to add those
> > two Fixes: tags into commit message for reference.
> >
> > > ---
> > >  fs/smb/client/cifssmb.c |  8 ++++----
> > >  fs/smb/client/smb1ops.c |  2 +-
> > >  fs/smb/client/smb2ops.c | 29 +++++++++++++++++++----------
> > >  3 files changed, 24 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
> > > index cfae2e918209..0ffc45aa5e2c 100644
> > > --- a/fs/smb/client/cifssmb.c
> > > +++ b/fs/smb/client/cifssmb.c
> > > @@ -1076,8 +1076,8 @@ SMBLegacyOpen(const unsigned int xid, struct cifs_tcon *tcon,
> > >       pSMB->OpenFlags |= cpu_to_le16(REQ_MORE_INFO);
> > >       pSMB->Mode = cpu_to_le16(access_flags_to_smbopen_mode(access_flags));
> > >       pSMB->Mode |= cpu_to_le16(0x40); /* deny none */
> > > -     /* set file as system file if special file such
> > > -        as fifo and server expecting SFU style and
> > > +     /* set file as system file if special file such as fifo,
> > > +      * socket, char or block and server expecting SFU style and
> > >          no Unix extensions */
> > >
> > >       if (create_options & CREATE_OPTION_SPECIAL)
> > > @@ -1193,8 +1193,8 @@ CIFS_open(const unsigned int xid, struct cifs_open_parms *oparms, int *oplock,
> > >       req->AllocationSize = 0;
> > >
> > >       /*
> > > -      * Set file as system file if special file such as fifo and server
> > > -      * expecting SFU style and no Unix extensions.
> > > +      * Set file as system file if special file such as fifo, socket, char
> > > +      * or block and server expecting SFU style and no Unix extensions.
> > >        */
> > >       if (create_options & CREATE_OPTION_SPECIAL)
> > >               req->FileAttributes = cpu_to_le32(ATTR_SYSTEM);
> > > diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
> > > index e1f2feb56f45..e03c91a49650 100644
> > > --- a/fs/smb/client/smb1ops.c
> > > +++ b/fs/smb/client/smb1ops.c
> > > @@ -1078,7 +1078,7 @@ cifs_make_node(unsigned int xid, struct inode *inode,
> > >       /*
> > >        * Check if mounted with mount parm 'sfu' mount parm.
> > >        * SFU emulation should work with all servers, but only
> > > -      * supports block and char device (no socket & fifo),
> > > +      * supports block and char device, socket & fifo,
> > >        * and was used by default in earlier versions of Windows
> > >        */
> > >       if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL))
> > > diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> > > index 9c2d065d3cc4..9e90672caf60 100644
> > > --- a/fs/smb/client/smb2ops.c
> > > +++ b/fs/smb/client/smb2ops.c
> > > @@ -5066,26 +5066,32 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
> > >       struct cifs_fid fid;
> > >       unsigned int bytes_written;
> > >       struct win_dev pdev = {};
> > > +     size_t pdev_len = 0;
> > >       struct kvec iov[2];
> > >       __u32 oplock = server->oplocks ? REQ_OPLOCK : 0;
> > >       int rc;
> > >
> > >       switch (mode & S_IFMT) {
> > >       case S_IFCHR:
> > > +             pdev_len = sizeof(pdev);
> > >               memcpy(pdev.type, "IntxCHR\0", 8);
> > >               pdev.major = cpu_to_le64(MAJOR(dev));
> > >               pdev.minor = cpu_to_le64(MINOR(dev));
> > >               break;
> > >       case S_IFBLK:
> > > +             pdev_len = sizeof(pdev);
> > >               memcpy(pdev.type, "IntxBLK\0", 8);
> > >               pdev.major = cpu_to_le64(MAJOR(dev));
> > >               pdev.minor = cpu_to_le64(MINOR(dev));
> > >               break;
> > >       case S_IFSOCK:
> > > -             strscpy(pdev.type, "LnxSOCK");
> > > +             /* SFU socket is system file with one zero byte */
> > > +             pdev_len = 1;
> > > +             pdev.type[0] = '\0';
> > >               break;
> > >       case S_IFIFO:
> > > -             strscpy(pdev.type, "LnxFIFO");
> > > +             /* SFU fifo is system file which is empty */
> > > +             pdev_len = 0;
> > >               break;
> > >       default:
> > >               return -EPERM;
> > > @@ -5100,14 +5106,17 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
> > >       if (rc)
> > >               return rc;
> > >
> > > -     io_parms.pid = current->tgid;
> > > -     io_parms.tcon = tcon;
> > > -     io_parms.length = sizeof(pdev);
> > > -     iov[1].iov_base = &pdev;
> > > -     iov[1].iov_len = sizeof(pdev);
> > > +     if (pdev_len > 0) {
> > > +             io_parms.pid = current->tgid;
> > > +             io_parms.tcon = tcon;
> > > +             io_parms.length = pdev_len;
> > > +             iov[1].iov_base = &pdev;
> > > +             iov[1].iov_len = pdev_len;
> > > +
> > > +             rc = server->ops->sync_write(xid, &fid, &io_parms,
> > > +                                          &bytes_written, iov, 1);
> > > +     }
> > >
> > > -     rc = server->ops->sync_write(xid, &fid, &io_parms,
> > > -                                  &bytes_written, iov, 1);
> > >       server->ops->close(xid, tcon, &fid);
> > >       return rc;
> > >  }
> > > @@ -5149,7 +5158,7 @@ static int smb2_make_node(unsigned int xid, struct inode *inode,
> > >       /*
> > >        * Check if mounted with mount parm 'sfu' mount parm.
> > >        * SFU emulation should work with all servers, but only
> > > -      * supports block and char device (no socket & fifo),
> > > +      * supports block and char device, socket & fifo,
> > >        * and was used by default in earlier versions of Windows
> > >        */
> > >       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) {
> > > --
> > > 2.20.1
> > >
> >
>
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files
  2024-09-13 22:14     ` Steve French
  2024-09-13 22:33       ` Steve French
@ 2024-09-13 22:42       ` Pali Rohár
  2024-09-14  6:21         ` Steve French
  1 sibling, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2024-09-13 22:42 UTC (permalink / raw)
  To: Steve French
  Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, linux-cifs,
	linux-kernel

On Friday 13 September 2024 17:14:22 Steve French wrote:
> How did you find the format of the FIFO and SOCK file types?  I

For fifo there are multiple sources on internet, but none of them is
normative. Everything is just what people have tried. For example this
old email on samba list:
https://lists.samba.org/archive/linux-cifs-client/2005-May/000856.html

Format of the socket I have figured out by creating it in Interix
subsystem and then dumped content of the file from Win32 subsystem.
Then I checked that it has also same format over older MS NFS server.
It was easier than trying to search for some documentation (which I have
not found).

> couldn't find any references to those so added two new types to allow
> current Linux to be able to create these (especially since they are
> opaque to the server and thus low risk).

I was searching over internet again and now I have found patent
https://patents.google.com/patent/US20090049459 which describe symlink
content:

#define NFS_SPECFILE_LNK_V1  0x014b4e4c78746e49 /* “IntxLNK” */

But does not describe other types.

> > +     case S_IFSOCK:
> > -             strscpy(pdev.type, "LnxSOCK");
> > +             /* SFU socket is system file with one zero byte */
> > +             pdev_len = 1;
> > +             pdev.type[0] = '\0';
> >               break;
> >       case S_IFIFO:
> > -             strscpy(pdev.type, "LnxFIFO");
> > +             /* SFU fifo is system file which is empty */
> > +             pdev_len = 0;
> 
> My worry about the suggested change above is that it is possible that
> we could accidentally match to an empty file.

I fully understand your concerns, but code in this patch is for creating
new fifos. Not recognizing existing fifos.

Code for recognizing existing fifos (=empty file with system attribute)
was not changed and is in Linux cifs client for a very long time.

And if nobody complained yet then this is probably not an issue.

You can create manually on server empty file, mark it as system and then
Linux cifs client even without these changes would recognize that file
as fifo.

> We intentionally added
> the two new dev.type fields for these to avoid collisions with other
> things (and since they are Linux specific).  It seems risky to have an
> empty file with the system attribute marked as a FIFO, and similarly a
> file with one byte null as Socket.   Since this is for only the Linux
> client to recognize, I wanted to do something safe for those file
> types less likely to be confusing (ie strings for Socket and FIFO that
> were similar in length and format to the other special files seemed
> intuitive) and "LnxFIFO" and LnxSOCK" were used as the tags in the
> file to reduce confusion ie the tags for those two start with "Lnx" -
> ie "something used for Linux client" not related to the original
> Interix (those begin with "Intx").

I see. Now I understand what are those types (as I have not seen them
before). It is somehow misleading if such "LnxFIFO" and LnxSOCK"
functionality is provided by SFU option, but is incompatible with MS SFU
and also with MS NFS server. And is also incompatible with older Linux
cifs clients (as they do not understand those Lnx types).

> Note that in the long run we hope to use reparse points by default in
> more servers to store special files like this but there are a few
> cases for unusual workloads that need special file support that would
> have to use sfu still.  The newer reparse tags that Windows uses "WSL"
> have the advantage that they require fewer roundtrips to query (since
> the file type is in the reparse tag).

Yes, new WSL tags seems to be better. Also SFU mount option is not
activated by default.

> Also noticed an interesting problem when mounted with "sfu" -
> "smbgetinfo filebasicinfo /mnt/fifo1" will hang (in sys_open).  Is
> that expected for a FIFO?

Reading from fifo sleep reading process until some other process write
data to fifo. This is how fifos are working. You can try it on local
filesystem (e.g. ext4 or tmpfs).

$ mkfifo fifo
$ strace cat fifo

You will see in strace output of cat something like:
...
openat(AT_FDCWD, "fifo", O_RDONLY

And once you write to that fifo by other process (e.g. echo abc > fifo)
then cat will unfreeze and continue:

                                        = 3
fstat(3, {st_mode=S_IFIFO|0644, st_size=0, ...}) = 0
fadvise64(3, 0, 0, POSIX_FADV_SEQUENTIAL) = -1 ESPIPE (Neprípustné nastavenie pozície)
mmap(NULL, 139264, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f17a34bb000
read(3, "abc\n", 131072)                = 4
...


> On Fri, Sep 13, 2024 at 3:07 PM Pali Rohár <pali@kernel.org> wrote:
> >
> > On Thursday 12 September 2024 14:05:47 Pali Rohár wrote:
> > > SFU-style fifo is empty file with system attribute set. This format is used
> > > by old Microsoft POSIX subsystem and later also by OpenNT/Interix subsystem
> > > (which replaced Microsoft POSIX subsystem and is part of Microsoft SFU).
> > >
> > > SFU-style socket is file which has system attribute set and file content is
> > > one zero byte. This format was introduced in Interix 3.0 subsystem, as part
> > > of the Microsoft SFU 3.0 and is used also by all later versions. Previous
> > > versions had no UNIX domain socket support.
> > >
> > > Currently when sfu mount option is specified then CIFS creates fifo and
> > > socket special files with some strange LnxSOCK or LnxFIFO content which is
> > > not compatible with Microsoft SFU and neither recognized by other SMB
> > > implementations which have some SFU support, including older Linux cifs
> > > implementations.
> > >
> > > So when sfu mount option is specified, create all fifo and socket special
> > > files compatible with SFU format to achieve SFU interop, as it is expected
> > > by sfu mount option.
> > >
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> >
> > Fixes: 72bc63f5e23a ("smb3: fix creating FIFOs when mounting with "sfu" mount option")
> > Fixes: 518549c120e6 ("cifs: fix creating sockets when using sfu mount options")
> >
> > I located commits which introduced those strange LnxSOCK or LnxFIFO
> > types which are not compatible with SFU. I would suggest to add those
> > two Fixes: tags into commit message for reference.
> >
> > > ---
> > >  fs/smb/client/cifssmb.c |  8 ++++----
> > >  fs/smb/client/smb1ops.c |  2 +-
> > >  fs/smb/client/smb2ops.c | 29 +++++++++++++++++++----------
> > >  3 files changed, 24 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
> > > index cfae2e918209..0ffc45aa5e2c 100644
> > > --- a/fs/smb/client/cifssmb.c
> > > +++ b/fs/smb/client/cifssmb.c
> > > @@ -1076,8 +1076,8 @@ SMBLegacyOpen(const unsigned int xid, struct cifs_tcon *tcon,
> > >       pSMB->OpenFlags |= cpu_to_le16(REQ_MORE_INFO);
> > >       pSMB->Mode = cpu_to_le16(access_flags_to_smbopen_mode(access_flags));
> > >       pSMB->Mode |= cpu_to_le16(0x40); /* deny none */
> > > -     /* set file as system file if special file such
> > > -        as fifo and server expecting SFU style and
> > > +     /* set file as system file if special file such as fifo,
> > > +      * socket, char or block and server expecting SFU style and
> > >          no Unix extensions */
> > >
> > >       if (create_options & CREATE_OPTION_SPECIAL)
> > > @@ -1193,8 +1193,8 @@ CIFS_open(const unsigned int xid, struct cifs_open_parms *oparms, int *oplock,
> > >       req->AllocationSize = 0;
> > >
> > >       /*
> > > -      * Set file as system file if special file such as fifo and server
> > > -      * expecting SFU style and no Unix extensions.
> > > +      * Set file as system file if special file such as fifo, socket, char
> > > +      * or block and server expecting SFU style and no Unix extensions.
> > >        */
> > >       if (create_options & CREATE_OPTION_SPECIAL)
> > >               req->FileAttributes = cpu_to_le32(ATTR_SYSTEM);
> > > diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
> > > index e1f2feb56f45..e03c91a49650 100644
> > > --- a/fs/smb/client/smb1ops.c
> > > +++ b/fs/smb/client/smb1ops.c
> > > @@ -1078,7 +1078,7 @@ cifs_make_node(unsigned int xid, struct inode *inode,
> > >       /*
> > >        * Check if mounted with mount parm 'sfu' mount parm.
> > >        * SFU emulation should work with all servers, but only
> > > -      * supports block and char device (no socket & fifo),
> > > +      * supports block and char device, socket & fifo,
> > >        * and was used by default in earlier versions of Windows
> > >        */
> > >       if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL))
> > > diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> > > index 9c2d065d3cc4..9e90672caf60 100644
> > > --- a/fs/smb/client/smb2ops.c
> > > +++ b/fs/smb/client/smb2ops.c
> > > @@ -5066,26 +5066,32 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
> > >       struct cifs_fid fid;
> > >       unsigned int bytes_written;
> > >       struct win_dev pdev = {};
> > > +     size_t pdev_len = 0;
> > >       struct kvec iov[2];
> > >       __u32 oplock = server->oplocks ? REQ_OPLOCK : 0;
> > >       int rc;
> > >
> > >       switch (mode & S_IFMT) {
> > >       case S_IFCHR:
> > > +             pdev_len = sizeof(pdev);
> > >               memcpy(pdev.type, "IntxCHR\0", 8);
> > >               pdev.major = cpu_to_le64(MAJOR(dev));
> > >               pdev.minor = cpu_to_le64(MINOR(dev));
> > >               break;
> > >       case S_IFBLK:
> > > +             pdev_len = sizeof(pdev);
> > >               memcpy(pdev.type, "IntxBLK\0", 8);
> > >               pdev.major = cpu_to_le64(MAJOR(dev));
> > >               pdev.minor = cpu_to_le64(MINOR(dev));
> > >               break;
> > >       case S_IFSOCK:
> > > -             strscpy(pdev.type, "LnxSOCK");
> > > +             /* SFU socket is system file with one zero byte */
> > > +             pdev_len = 1;
> > > +             pdev.type[0] = '\0';
> > >               break;
> > >       case S_IFIFO:
> > > -             strscpy(pdev.type, "LnxFIFO");
> > > +             /* SFU fifo is system file which is empty */
> > > +             pdev_len = 0;
> > >               break;
> > >       default:
> > >               return -EPERM;
> > > @@ -5100,14 +5106,17 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
> > >       if (rc)
> > >               return rc;
> > >
> > > -     io_parms.pid = current->tgid;
> > > -     io_parms.tcon = tcon;
> > > -     io_parms.length = sizeof(pdev);
> > > -     iov[1].iov_base = &pdev;
> > > -     iov[1].iov_len = sizeof(pdev);
> > > +     if (pdev_len > 0) {
> > > +             io_parms.pid = current->tgid;
> > > +             io_parms.tcon = tcon;
> > > +             io_parms.length = pdev_len;
> > > +             iov[1].iov_base = &pdev;
> > > +             iov[1].iov_len = pdev_len;
> > > +
> > > +             rc = server->ops->sync_write(xid, &fid, &io_parms,
> > > +                                          &bytes_written, iov, 1);
> > > +     }
> > >
> > > -     rc = server->ops->sync_write(xid, &fid, &io_parms,
> > > -                                  &bytes_written, iov, 1);
> > >       server->ops->close(xid, tcon, &fid);
> > >       return rc;
> > >  }
> > > @@ -5149,7 +5158,7 @@ static int smb2_make_node(unsigned int xid, struct inode *inode,
> > >       /*
> > >        * Check if mounted with mount parm 'sfu' mount parm.
> > >        * SFU emulation should work with all servers, but only
> > > -      * supports block and char device (no socket & fifo),
> > > +      * supports block and char device, socket & fifo,
> > >        * and was used by default in earlier versions of Windows
> > >        */
> > >       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) {
> > > --
> > > 2.20.1
> > >
> >
> 
> 
> -- 
> Thanks,
> 
> Steve

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files
  2024-09-13 22:33       ` Steve French
@ 2024-09-13 22:45         ` Pali Rohár
  0 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2024-09-13 22:45 UTC (permalink / raw)
  To: Steve French
  Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, linux-cifs,
	linux-kernel

Yes, but it was enabled also before this patch series.

Now I have checked that even old Linux 4.19 cifs kernel client detects
empty file with system attribute as fifo. This functionality is there
for a long time. Probably since first cifs/sfu version.

On Friday 13 September 2024 17:33:22 Steve French wrote:
> One other thing which worried me a bit is that I confirmed that an
> empty file with the system attribute set was matched as a FIFO file
> type (when you mount with "sfu") - this seems risky.
> 
> On Fri, Sep 13, 2024 at 5:14 PM Steve French <smfrench@gmail.com> wrote:
> >
> > How did you find the format of the FIFO and SOCK file types?  I
> > couldn't find any references to those so added two new types to allow
> > current Linux to be able to create these (especially since they are
> > opaque to the server and thus low risk).
> >
> > > +     case S_IFSOCK:
> > > -             strscpy(pdev.type, "LnxSOCK");
> > > +             /* SFU socket is system file with one zero byte */
> > > +             pdev_len = 1;
> > > +             pdev.type[0] = '\0';
> > >               break;
> > >       case S_IFIFO:
> > > -             strscpy(pdev.type, "LnxFIFO");
> > > +             /* SFU fifo is system file which is empty */
> > > +             pdev_len = 0;
> >
> > My worry about the suggested change above is that it is possible that
> > we could accidentally match to an empty file. We intentionally added
> > the two new dev.type fields for these to avoid collisions with other
> > things (and since they are Linux specific).  It seems risky to have an
> > empty file with the system attribute marked as a FIFO, and similarly a
> > file with one byte null as Socket.   Since this is for only the Linux
> > client to recognize, I wanted to do something safe for those file
> > types less likely to be confusing (ie strings for Socket and FIFO that
> > were similar in length and format to the other special files seemed
> > intuitive) and "LnxFIFO" and LnxSOCK" were used as the tags in the
> > file to reduce confusion ie the tags for those two start with "Lnx" -
> > ie "something used for Linux client" not related to the original
> > Interix (those begin with "Intx").
> >
> > Note that in the long run we hope to use reparse points by default in
> > more servers to store special files like this but there are a few
> > cases for unusual workloads that need special file support that would
> > have to use sfu still.  The newer reparse tags that Windows uses "WSL"
> > have the advantage that they require fewer roundtrips to query (since
> > the file type is in the reparse tag).
> >
> > Also noticed an interesting problem when mounted with "sfu" -
> > "smbgetinfo filebasicinfo /mnt/fifo1" will hang (in sys_open).  Is
> > that expected for a FIFO?
> >
> > On Fri, Sep 13, 2024 at 3:07 PM Pali Rohár <pali@kernel.org> wrote:
> > >
> > > On Thursday 12 September 2024 14:05:47 Pali Rohár wrote:
> > > > SFU-style fifo is empty file with system attribute set. This format is used
> > > > by old Microsoft POSIX subsystem and later also by OpenNT/Interix subsystem
> > > > (which replaced Microsoft POSIX subsystem and is part of Microsoft SFU).
> > > >
> > > > SFU-style socket is file which has system attribute set and file content is
> > > > one zero byte. This format was introduced in Interix 3.0 subsystem, as part
> > > > of the Microsoft SFU 3.0 and is used also by all later versions. Previous
> > > > versions had no UNIX domain socket support.
> > > >
> > > > Currently when sfu mount option is specified then CIFS creates fifo and
> > > > socket special files with some strange LnxSOCK or LnxFIFO content which is
> > > > not compatible with Microsoft SFU and neither recognized by other SMB
> > > > implementations which have some SFU support, including older Linux cifs
> > > > implementations.
> > > >
> > > > So when sfu mount option is specified, create all fifo and socket special
> > > > files compatible with SFU format to achieve SFU interop, as it is expected
> > > > by sfu mount option.
> > > >
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > >
> > > Fixes: 72bc63f5e23a ("smb3: fix creating FIFOs when mounting with "sfu" mount option")
> > > Fixes: 518549c120e6 ("cifs: fix creating sockets when using sfu mount options")
> > >
> > > I located commits which introduced those strange LnxSOCK or LnxFIFO
> > > types which are not compatible with SFU. I would suggest to add those
> > > two Fixes: tags into commit message for reference.
> > >
> > > > ---
> > > >  fs/smb/client/cifssmb.c |  8 ++++----
> > > >  fs/smb/client/smb1ops.c |  2 +-
> > > >  fs/smb/client/smb2ops.c | 29 +++++++++++++++++++----------
> > > >  3 files changed, 24 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
> > > > index cfae2e918209..0ffc45aa5e2c 100644
> > > > --- a/fs/smb/client/cifssmb.c
> > > > +++ b/fs/smb/client/cifssmb.c
> > > > @@ -1076,8 +1076,8 @@ SMBLegacyOpen(const unsigned int xid, struct cifs_tcon *tcon,
> > > >       pSMB->OpenFlags |= cpu_to_le16(REQ_MORE_INFO);
> > > >       pSMB->Mode = cpu_to_le16(access_flags_to_smbopen_mode(access_flags));
> > > >       pSMB->Mode |= cpu_to_le16(0x40); /* deny none */
> > > > -     /* set file as system file if special file such
> > > > -        as fifo and server expecting SFU style and
> > > > +     /* set file as system file if special file such as fifo,
> > > > +      * socket, char or block and server expecting SFU style and
> > > >          no Unix extensions */
> > > >
> > > >       if (create_options & CREATE_OPTION_SPECIAL)
> > > > @@ -1193,8 +1193,8 @@ CIFS_open(const unsigned int xid, struct cifs_open_parms *oparms, int *oplock,
> > > >       req->AllocationSize = 0;
> > > >
> > > >       /*
> > > > -      * Set file as system file if special file such as fifo and server
> > > > -      * expecting SFU style and no Unix extensions.
> > > > +      * Set file as system file if special file such as fifo, socket, char
> > > > +      * or block and server expecting SFU style and no Unix extensions.
> > > >        */
> > > >       if (create_options & CREATE_OPTION_SPECIAL)
> > > >               req->FileAttributes = cpu_to_le32(ATTR_SYSTEM);
> > > > diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
> > > > index e1f2feb56f45..e03c91a49650 100644
> > > > --- a/fs/smb/client/smb1ops.c
> > > > +++ b/fs/smb/client/smb1ops.c
> > > > @@ -1078,7 +1078,7 @@ cifs_make_node(unsigned int xid, struct inode *inode,
> > > >       /*
> > > >        * Check if mounted with mount parm 'sfu' mount parm.
> > > >        * SFU emulation should work with all servers, but only
> > > > -      * supports block and char device (no socket & fifo),
> > > > +      * supports block and char device, socket & fifo,
> > > >        * and was used by default in earlier versions of Windows
> > > >        */
> > > >       if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL))
> > > > diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> > > > index 9c2d065d3cc4..9e90672caf60 100644
> > > > --- a/fs/smb/client/smb2ops.c
> > > > +++ b/fs/smb/client/smb2ops.c
> > > > @@ -5066,26 +5066,32 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
> > > >       struct cifs_fid fid;
> > > >       unsigned int bytes_written;
> > > >       struct win_dev pdev = {};
> > > > +     size_t pdev_len = 0;
> > > >       struct kvec iov[2];
> > > >       __u32 oplock = server->oplocks ? REQ_OPLOCK : 0;
> > > >       int rc;
> > > >
> > > >       switch (mode & S_IFMT) {
> > > >       case S_IFCHR:
> > > > +             pdev_len = sizeof(pdev);
> > > >               memcpy(pdev.type, "IntxCHR\0", 8);
> > > >               pdev.major = cpu_to_le64(MAJOR(dev));
> > > >               pdev.minor = cpu_to_le64(MINOR(dev));
> > > >               break;
> > > >       case S_IFBLK:
> > > > +             pdev_len = sizeof(pdev);
> > > >               memcpy(pdev.type, "IntxBLK\0", 8);
> > > >               pdev.major = cpu_to_le64(MAJOR(dev));
> > > >               pdev.minor = cpu_to_le64(MINOR(dev));
> > > >               break;
> > > >       case S_IFSOCK:
> > > > -             strscpy(pdev.type, "LnxSOCK");
> > > > +             /* SFU socket is system file with one zero byte */
> > > > +             pdev_len = 1;
> > > > +             pdev.type[0] = '\0';
> > > >               break;
> > > >       case S_IFIFO:
> > > > -             strscpy(pdev.type, "LnxFIFO");
> > > > +             /* SFU fifo is system file which is empty */
> > > > +             pdev_len = 0;
> > > >               break;
> > > >       default:
> > > >               return -EPERM;
> > > > @@ -5100,14 +5106,17 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
> > > >       if (rc)
> > > >               return rc;
> > > >
> > > > -     io_parms.pid = current->tgid;
> > > > -     io_parms.tcon = tcon;
> > > > -     io_parms.length = sizeof(pdev);
> > > > -     iov[1].iov_base = &pdev;
> > > > -     iov[1].iov_len = sizeof(pdev);
> > > > +     if (pdev_len > 0) {
> > > > +             io_parms.pid = current->tgid;
> > > > +             io_parms.tcon = tcon;
> > > > +             io_parms.length = pdev_len;
> > > > +             iov[1].iov_base = &pdev;
> > > > +             iov[1].iov_len = pdev_len;
> > > > +
> > > > +             rc = server->ops->sync_write(xid, &fid, &io_parms,
> > > > +                                          &bytes_written, iov, 1);
> > > > +     }
> > > >
> > > > -     rc = server->ops->sync_write(xid, &fid, &io_parms,
> > > > -                                  &bytes_written, iov, 1);
> > > >       server->ops->close(xid, tcon, &fid);
> > > >       return rc;
> > > >  }
> > > > @@ -5149,7 +5158,7 @@ static int smb2_make_node(unsigned int xid, struct inode *inode,
> > > >       /*
> > > >        * Check if mounted with mount parm 'sfu' mount parm.
> > > >        * SFU emulation should work with all servers, but only
> > > > -      * supports block and char device (no socket & fifo),
> > > > +      * supports block and char device, socket & fifo,
> > > >        * and was used by default in earlier versions of Windows
> > > >        */
> > > >       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) {
> > > > --
> > > > 2.20.1
> > > >
> > >
> >
> >
> > --
> > Thanks,
> >
> > Steve
> 
> 
> 
> -- 
> Thanks,
> 
> Steve

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files
  2024-09-13 22:42       ` Pali Rohár
@ 2024-09-14  6:21         ` Steve French
  2024-09-14  8:17           ` Pali Rohár
  0 siblings, 1 reply; 30+ messages in thread
From: Steve French @ 2024-09-14  6:21 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, linux-cifs,
	linux-kernel

On Fri, Sep 13, 2024 at 5:42 PM Pali Rohár <pali@kernel.org> wrote:
>
> On Friday 13 September 2024 17:14:22 Steve French wrote:
> > How did you find the format of the FIFO and SOCK file types?  I
>
> For fifo there are multiple sources on internet, but none of them is
> normative. Everything is just what people have tried. For example this
> old email on samba list:
> https://lists.samba.org/archive/linux-cifs-client/2005-May/000856.html
>
> Format of the socket I have figured out by creating it in Interix
> subsystem and then dumped content of the file from Win32 subsystem.
> Then I checked that it has also same format over older MS NFS server.
> It was easier than trying to search for some documentation (which I have
> not found).
>
> > couldn't find any references to those so added two new types to allow
> > current Linux to be able to create these (especially since they are
> > opaque to the server and thus low risk).
>
> I was searching over internet again and now I have found patent
> https://patents.google.com/patent/US20090049459 which describe symlink
> content:
>
> #define NFS_SPECFILE_LNK_V1  0x014b4e4c78746e49 /* “IntxLNK” */
>
> But does not describe other types.
>
> > > +     case S_IFSOCK:
> > > -             strscpy(pdev.type, "LnxSOCK");
> > > +             /* SFU socket is system file with one zero byte */
> > > +             pdev_len = 1;
> > > +             pdev.type[0] = '\0';
> > >               break;
> > >       case S_IFIFO:
> > > -             strscpy(pdev.type, "LnxFIFO");
> > > +             /* SFU fifo is system file which is empty */
> > > +             pdev_len = 0;
> >
> > My worry about the suggested change above is that it is possible that
> > we could accidentally match to an empty file.
>
> I fully understand your concerns, but code in this patch is for creating
> new fifos. Not recognizing existing fifos.
>
> Code for recognizing existing fifos (=empty file with system attribute)
> was not changed and is in Linux cifs client for a very long time.
<>
> > We intentionally added
> > the two new dev.type fields for these to avoid collisions with other
> > things (and since they are Linux specific).  It seems risky to have an
> > empty file with the system attribute marked as a FIFO, and similarly a
> > file with one byte null as Socket.   Since this is for only the Linux
> > client to recognize, I wanted to do something safe for those file
> > types less likely to be confusing (ie strings for Socket and FIFO that
> > were similar in length and format to the other special files seemed
> > intuitive) and "LnxFIFO" and LnxSOCK" were used as the tags in the
> > file to reduce confusion ie the tags for those two start with "Lnx" -
> > ie "something used for Linux client" not related to the original
> > Interix (those begin with "Intx").
>
> I see. Now I understand what are those types (as I have not seen them
> before). It is somehow misleading if such "LnxFIFO" and LnxSOCK"
> functionality is provided by SFU option, but is incompatible with MS SFU
> and also with MS NFS server. And is also incompatible with older Linux
> cifs clients (as they do not understand those Lnx types).

I am not as worried about FIFO and SOCK type being recognized by
older servers (since almost every use case for them would be for them
to be seen (only) by the client - e.g. for mounts to servers that
don't implement reparse points yet), and since they are less
common file types I am willing to let them be unrecognized by
old clients (we can tag them for stable if older distros don't have
them),  but I am concerned about allowing "current clients" to
create empty files for an unusual purpose which could be
confusing/non-intuitive.

And since this change (at least the one to allow FIFOs to be created with "sfu"
has been in mainline for a year and also since it uses a more intuitive tag
("LnxFIFO") than the empty one used by very old Windows) the only
clients who would have created these would be already using this newer tag
(older Linux clients couldn't have created such files - there seems more
risk of regression with reverting the change than with continuing with
the Linux client specific tag (at least to the one for FIFOs
since that has been in much longer than the socket one which is recent)

Will discuss with others - opinions welcome.

There is an upcoming SMB3.1.1 test event coming up next week (and the annual
Storage Developer Conference too) so I can see if others have opinions one
way or another on whether to move to empty (or 1 byte) files for
creating fifos/sockets

> > Note that in the long run we hope to use reparse points by default in
> > more servers to store special files like this but there are a few
> > cases for unusual workloads that need special file support that would
> > have to use sfu still.  The newer reparse tags that Windows uses "WSL"
> > have the advantage that they require fewer roundtrips to query (since
> > the file type is in the reparse tag).
>
> Yes, new WSL tags seems to be better. Also SFU mount option is not
> activated by default.
>
> > Also noticed an interesting problem when mounted with "sfu" -
> > "smbgetinfo filebasicinfo /mnt/fifo1" will hang (in sys_open).  Is
> > that expected for a FIFO?
>
> Reading from fifo sleep reading process until some other process write
> data to fifo. This is how fifos are working. You can try it on local
> filesystem (e.g. ext4 or tmpfs).

makes sense - thx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files
  2024-09-14  6:21         ` Steve French
@ 2024-09-14  8:17           ` Pali Rohár
  2024-09-15 17:41             ` Pali Rohár
  0 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2024-09-14  8:17 UTC (permalink / raw)
  To: Steve French
  Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, linux-cifs,
	linux-kernel

On Saturday 14 September 2024 01:21:17 Steve French wrote:
> On Fri, Sep 13, 2024 at 5:42 PM Pali Rohár <pali@kernel.org> wrote:
> >
> > On Friday 13 September 2024 17:14:22 Steve French wrote:
> > > How did you find the format of the FIFO and SOCK file types?  I
> >
> > For fifo there are multiple sources on internet, but none of them is
> > normative. Everything is just what people have tried. For example this
> > old email on samba list:
> > https://lists.samba.org/archive/linux-cifs-client/2005-May/000856.html
> >
> > Format of the socket I have figured out by creating it in Interix
> > subsystem and then dumped content of the file from Win32 subsystem.
> > Then I checked that it has also same format over older MS NFS server.
> > It was easier than trying to search for some documentation (which I have
> > not found).
> >
> > > couldn't find any references to those so added two new types to allow
> > > current Linux to be able to create these (especially since they are
> > > opaque to the server and thus low risk).
> >
> > I was searching over internet again and now I have found patent
> > https://patents.google.com/patent/US20090049459 which describe symlink
> > content:
> >
> > #define NFS_SPECFILE_LNK_V1  0x014b4e4c78746e49 /* “IntxLNK” */
> >
> > But does not describe other types.
> >
> > > > +     case S_IFSOCK:
> > > > -             strscpy(pdev.type, "LnxSOCK");
> > > > +             /* SFU socket is system file with one zero byte */
> > > > +             pdev_len = 1;
> > > > +             pdev.type[0] = '\0';
> > > >               break;
> > > >       case S_IFIFO:
> > > > -             strscpy(pdev.type, "LnxFIFO");
> > > > +             /* SFU fifo is system file which is empty */
> > > > +             pdev_len = 0;
> > >
> > > My worry about the suggested change above is that it is possible that
> > > we could accidentally match to an empty file.
> >
> > I fully understand your concerns, but code in this patch is for creating
> > new fifos. Not recognizing existing fifos.
> >
> > Code for recognizing existing fifos (=empty file with system attribute)
> > was not changed and is in Linux cifs client for a very long time.
> <>
> > > We intentionally added
> > > the two new dev.type fields for these to avoid collisions with other
> > > things (and since they are Linux specific).  It seems risky to have an
> > > empty file with the system attribute marked as a FIFO, and similarly a
> > > file with one byte null as Socket.   Since this is for only the Linux
> > > client to recognize, I wanted to do something safe for those file
> > > types less likely to be confusing (ie strings for Socket and FIFO that
> > > were similar in length and format to the other special files seemed
> > > intuitive) and "LnxFIFO" and LnxSOCK" were used as the tags in the
> > > file to reduce confusion ie the tags for those two start with "Lnx" -
> > > ie "something used for Linux client" not related to the original
> > > Interix (those begin with "Intx").
> >
> > I see. Now I understand what are those types (as I have not seen them
> > before). It is somehow misleading if such "LnxFIFO" and LnxSOCK"
> > functionality is provided by SFU option, but is incompatible with MS SFU
> > and also with MS NFS server. And is also incompatible with older Linux
> > cifs clients (as they do not understand those Lnx types).
> 
> I am not as worried about FIFO and SOCK type being recognized by
> older servers (since almost every use case for them would be for them
> to be seen (only) by the client - e.g. for mounts to servers that
> don't implement reparse points yet), and since they are less
> common file types I am willing to let them be unrecognized by
> old clients (we can tag them for stable if older distros don't have
> them),

This is quite pity for old clients, to break existing interoperability.
At least I see sfu as an compatibility option either for ecosystem with
old clients, or option where server itself does not support reparse
points.

> but I am concerned about allowing "current clients" to
> create empty files for an unusual purpose which could be
> confusing/non-intuitive.

I understand this concern. I thought that this should not be an issue
because files are created with system attribute which is not common for
normal/ordinary usage (system attribute could be less confusing) and
also because this format, at least for fifo is used and understood by
many SW for about 30 years.

> And since this change (at least the one to allow FIFOs to be created with "sfu"
> has been in mainline for a year and also since it uses a more intuitive tag
> ("LnxFIFO") than the empty one used by very old Windows) the only
> clients who would have created these would be already using this newer tag
> (older Linux clients couldn't have created such files - there seems more
> risk of regression with reverting the change than with continuing with
> the Linux client specific tag (at least to the one for FIFOs
> since that has been in much longer than the socket one which is recent)

This kind of stuff is lot of times used on LTS/stable linux
distributions and new kernel to these users/admins do not have to be
delivered yet. Mostly it takes 2-3 years after release. Look for example
at RHEL cycles.

I'm looking on this from opposite perspective. I see this an regression
in -o sfu option that after upgrading from previous LTS version to new,
-o sfu stopped to be compatible with SFU-style fifos.

But your point is valid. But maybe it is not an issue because users
do not have updated yet to new version?

> Will discuss with others - opinions welcome.
> 
> There is an upcoming SMB3.1.1 test event coming up next week (and the annual
> Storage Developer Conference too) so I can see if others have opinions one
> way or another on whether to move to empty (or 1 byte) files for
> creating fifos/sockets

Ok, perfect, let me know then about the result.

> > > Note that in the long run we hope to use reparse points by default in
> > > more servers to store special files like this but there are a few
> > > cases for unusual workloads that need special file support that would
> > > have to use sfu still.  The newer reparse tags that Windows uses "WSL"
> > > have the advantage that they require fewer roundtrips to query (since
> > > the file type is in the reparse tag).
> >
> > Yes, new WSL tags seems to be better. Also SFU mount option is not
> > activated by default.
> >
> > > Also noticed an interesting problem when mounted with "sfu" -
> > > "smbgetinfo filebasicinfo /mnt/fifo1" will hang (in sys_open).  Is
> > > that expected for a FIFO?
> >
> > Reading from fifo sleep reading process until some other process write
> > data to fifo. This is how fifos are working. You can try it on local
> > filesystem (e.g. ext4 or tmpfs).
> 
> makes sense - thx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files
  2024-09-14  8:17           ` Pali Rohár
@ 2024-09-15 17:41             ` Pali Rohár
       [not found]               ` <CAH2r5muXcyMxc=F2WsTtwQyKZ9TL64TWEBzX7bXJqZky2g0TzA@mail.gmail.com>
  0 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2024-09-15 17:41 UTC (permalink / raw)
  To: Steve French
  Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, linux-cifs,
	linux-kernel

Hello Steve,

I have another argument for the empty system file as fifo over the
file with "LnxFIFO" content.

Now I have figured out that even the latest Windows Server 2022 version
provides interoperability of FIFOs in SFU format with Windows NFS 4.1
Server. So if you configure on Windows Server 2022 one share which is
exported over SMB and also NFS at the same time, and over SMB you create
SFU-style fifo, then Windows NFS4.1 server recognize it and properly
reports nfs4type as NFS4FIFO for NFSv4.1 client.

So this SFU FIFO style is not only for old clients and servers, but
still relevant and useful even for latest Windows Server.

And same applies for named sockets.

For testing this scenario it is enough to use just trial version of
latest Windows Server from:
https://go.microsoft.com/fwlink/p/?linkid=2208182
https://go.microsoft.com/fwlink/p/?Linkid=2195280

For setting NFS4.1 and SMB share named "test" I used these cmd commands:

  dism /Online /Enable-Feature /All /FeatureName:ServerForNFS-Infrastructure
  md C:\test
  icacls C:\test /grant Everyone:(OI)(CI)F /T
  nfsshare test=C:\test -o rw unmapped=yes
  net share test=C:\test /grant:Everyone,FULL

(for everyone who is going to reproduce this scenario, beware that new
Windows servers use by default powershell, so first launch cmd.exe then
copy+paste those commands)

Pali

On Saturday 14 September 2024 10:17:42 Pali Rohár wrote:
> On Saturday 14 September 2024 01:21:17 Steve French wrote:
> > On Fri, Sep 13, 2024 at 5:42 PM Pali Rohár <pali@kernel.org> wrote:
> > >
> > > On Friday 13 September 2024 17:14:22 Steve French wrote:
> > > > How did you find the format of the FIFO and SOCK file types?  I
> > >
> > > For fifo there are multiple sources on internet, but none of them is
> > > normative. Everything is just what people have tried. For example this
> > > old email on samba list:
> > > https://lists.samba.org/archive/linux-cifs-client/2005-May/000856.html
> > >
> > > Format of the socket I have figured out by creating it in Interix
> > > subsystem and then dumped content of the file from Win32 subsystem.
> > > Then I checked that it has also same format over older MS NFS server.
> > > It was easier than trying to search for some documentation (which I have
> > > not found).
> > >
> > > > couldn't find any references to those so added two new types to allow
> > > > current Linux to be able to create these (especially since they are
> > > > opaque to the server and thus low risk).
> > >
> > > I was searching over internet again and now I have found patent
> > > https://patents.google.com/patent/US20090049459 which describe symlink
> > > content:
> > >
> > > #define NFS_SPECFILE_LNK_V1  0x014b4e4c78746e49 /* “IntxLNK” */
> > >
> > > But does not describe other types.
> > >
> > > > > +     case S_IFSOCK:
> > > > > -             strscpy(pdev.type, "LnxSOCK");
> > > > > +             /* SFU socket is system file with one zero byte */
> > > > > +             pdev_len = 1;
> > > > > +             pdev.type[0] = '\0';
> > > > >               break;
> > > > >       case S_IFIFO:
> > > > > -             strscpy(pdev.type, "LnxFIFO");
> > > > > +             /* SFU fifo is system file which is empty */
> > > > > +             pdev_len = 0;
> > > >
> > > > My worry about the suggested change above is that it is possible that
> > > > we could accidentally match to an empty file.
> > >
> > > I fully understand your concerns, but code in this patch is for creating
> > > new fifos. Not recognizing existing fifos.
> > >
> > > Code for recognizing existing fifos (=empty file with system attribute)
> > > was not changed and is in Linux cifs client for a very long time.
> > <>
> > > > We intentionally added
> > > > the two new dev.type fields for these to avoid collisions with other
> > > > things (and since they are Linux specific).  It seems risky to have an
> > > > empty file with the system attribute marked as a FIFO, and similarly a
> > > > file with one byte null as Socket.   Since this is for only the Linux
> > > > client to recognize, I wanted to do something safe for those file
> > > > types less likely to be confusing (ie strings for Socket and FIFO that
> > > > were similar in length and format to the other special files seemed
> > > > intuitive) and "LnxFIFO" and LnxSOCK" were used as the tags in the
> > > > file to reduce confusion ie the tags for those two start with "Lnx" -
> > > > ie "something used for Linux client" not related to the original
> > > > Interix (those begin with "Intx").
> > >
> > > I see. Now I understand what are those types (as I have not seen them
> > > before). It is somehow misleading if such "LnxFIFO" and LnxSOCK"
> > > functionality is provided by SFU option, but is incompatible with MS SFU
> > > and also with MS NFS server. And is also incompatible with older Linux
> > > cifs clients (as they do not understand those Lnx types).
> > 
> > I am not as worried about FIFO and SOCK type being recognized by
> > older servers (since almost every use case for them would be for them
> > to be seen (only) by the client - e.g. for mounts to servers that
> > don't implement reparse points yet), and since they are less
> > common file types I am willing to let them be unrecognized by
> > old clients (we can tag them for stable if older distros don't have
> > them),
> 
> This is quite pity for old clients, to break existing interoperability.
> At least I see sfu as an compatibility option either for ecosystem with
> old clients, or option where server itself does not support reparse
> points.
> 
> > but I am concerned about allowing "current clients" to
> > create empty files for an unusual purpose which could be
> > confusing/non-intuitive.
> 
> I understand this concern. I thought that this should not be an issue
> because files are created with system attribute which is not common for
> normal/ordinary usage (system attribute could be less confusing) and
> also because this format, at least for fifo is used and understood by
> many SW for about 30 years.
> 
> > And since this change (at least the one to allow FIFOs to be created with "sfu"
> > has been in mainline for a year and also since it uses a more intuitive tag
> > ("LnxFIFO") than the empty one used by very old Windows) the only
> > clients who would have created these would be already using this newer tag
> > (older Linux clients couldn't have created such files - there seems more
> > risk of regression with reverting the change than with continuing with
> > the Linux client specific tag (at least to the one for FIFOs
> > since that has been in much longer than the socket one which is recent)
> 
> This kind of stuff is lot of times used on LTS/stable linux
> distributions and new kernel to these users/admins do not have to be
> delivered yet. Mostly it takes 2-3 years after release. Look for example
> at RHEL cycles.
> 
> I'm looking on this from opposite perspective. I see this an regression
> in -o sfu option that after upgrading from previous LTS version to new,
> -o sfu stopped to be compatible with SFU-style fifos.
> 
> But your point is valid. But maybe it is not an issue because users
> do not have updated yet to new version?
> 
> > Will discuss with others - opinions welcome.
> > 
> > There is an upcoming SMB3.1.1 test event coming up next week (and the annual
> > Storage Developer Conference too) so I can see if others have opinions one
> > way or another on whether to move to empty (or 1 byte) files for
> > creating fifos/sockets
> 
> Ok, perfect, let me know then about the result.
> 
> > > > Note that in the long run we hope to use reparse points by default in
> > > > more servers to store special files like this but there are a few
> > > > cases for unusual workloads that need special file support that would
> > > > have to use sfu still.  The newer reparse tags that Windows uses "WSL"
> > > > have the advantage that they require fewer roundtrips to query (since
> > > > the file type is in the reparse tag).
> > >
> > > Yes, new WSL tags seems to be better. Also SFU mount option is not
> > > activated by default.
> > >
> > > > Also noticed an interesting problem when mounted with "sfu" -
> > > > "smbgetinfo filebasicinfo /mnt/fifo1" will hang (in sys_open).  Is
> > > > that expected for a FIFO?
> > >
> > > Reading from fifo sleep reading process until some other process write
> > > data to fifo. This is how fifos are working. You can try it on local
> > > filesystem (e.g. ext4 or tmpfs).
> > 
> > makes sense - thx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files
       [not found]               ` <CAH2r5muXcyMxc=F2WsTtwQyKZ9TL64TWEBzX7bXJqZky2g0TzA@mail.gmail.com>
@ 2024-09-15 17:48                 ` Pali Rohár
  2024-09-16 16:23                   ` Pali Rohár
  0 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2024-09-15 17:48 UTC (permalink / raw)
  To: Steve French; +Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, CIFS, LKML

It works for this new 2022 version and also for old 2012 version. So my
personal guess is yes, that it would work also for 2019 (I do not see
reason why something would not work for version in-the-middle). If
needed, I may try to do this test against 2019 version during next or
another weekend.

On Sunday 15 September 2024 12:42:27 Steve French wrote:
> Do you know if this is the case for windows server 2019
> 
> On Sun, Sep 15, 2024, 12:41 PM Pali Rohár <pali@kernel.org> wrote:
> 
> > Hello Steve,
> >
> > I have another argument for the empty system file as fifo over the
> > file with "LnxFIFO" content.
> >
> > Now I have figured out that even the latest Windows Server 2022 version
> > provides interoperability of FIFOs in SFU format with Windows NFS 4.1
> > Server. So if you configure on Windows Server 2022 one share which is
> > exported over SMB and also NFS at the same time, and over SMB you create
> > SFU-style fifo, then Windows NFS4.1 server recognize it and properly
> > reports nfs4type as NFS4FIFO for NFSv4.1 client.
> >
> > So this SFU FIFO style is not only for old clients and servers, but
> > still relevant and useful even for latest Windows Server.
> >
> > And same applies for named sockets.
> >
> > For testing this scenario it is enough to use just trial version of
> > latest Windows Server from:
> > https://go.microsoft.com/fwlink/p/?linkid=2208182
> > https://go.microsoft.com/fwlink/p/?Linkid=2195280
> >
> > For setting NFS4.1 and SMB share named "test" I used these cmd commands:
> >
> >   dism /Online /Enable-Feature /All
> > /FeatureName:ServerForNFS-Infrastructure
> >   md C:\test
> >   icacls C:\test /grant Everyone:(OI)(CI)F /T
> >   nfsshare test=C:\test -o rw unmapped=yes
> >   net share test=C:\test /grant:Everyone,FULL
> >
> > (for everyone who is going to reproduce this scenario, beware that new
> > Windows servers use by default powershell, so first launch cmd.exe then
> > copy+paste those commands)
> >
> > Pali
> >
> > On Saturday 14 September 2024 10:17:42 Pali Rohár wrote:
> > > On Saturday 14 September 2024 01:21:17 Steve French wrote:
> > > > On Fri, Sep 13, 2024 at 5:42 PM Pali Rohár <pali@kernel.org> wrote:
> > > > >
> > > > > On Friday 13 September 2024 17:14:22 Steve French wrote:
> > > > > > How did you find the format of the FIFO and SOCK file types?  I
> > > > >
> > > > > For fifo there are multiple sources on internet, but none of them is
> > > > > normative. Everything is just what people have tried. For example
> > this
> > > > > old email on samba list:
> > > > >
> > https://lists.samba.org/archive/linux-cifs-client/2005-May/000856.html
> > > > >
> > > > > Format of the socket I have figured out by creating it in Interix
> > > > > subsystem and then dumped content of the file from Win32 subsystem.
> > > > > Then I checked that it has also same format over older MS NFS server.
> > > > > It was easier than trying to search for some documentation (which I
> > have
> > > > > not found).
> > > > >
> > > > > > couldn't find any references to those so added two new types to
> > allow
> > > > > > current Linux to be able to create these (especially since they are
> > > > > > opaque to the server and thus low risk).
> > > > >
> > > > > I was searching over internet again and now I have found patent
> > > > > https://patents.google.com/patent/US20090049459 which describe
> > symlink
> > > > > content:
> > > > >
> > > > > #define NFS_SPECFILE_LNK_V1 0x014b4e4c78746e49 /* “IntxLNK” */
> > > > >
> > > > > But does not describe other types.
> > > > >
> > > > > > > +     case S_IFSOCK:
> > > > > > > -             strscpy(pdev.type, "LnxSOCK");
> > > > > > > +             /* SFU socket is system file with one zero byte */
> > > > > > > +             pdev_len = 1;
> > > > > > > +             pdev.type[0] = '\0';
> > > > > > >               break;
> > > > > > >       case S_IFIFO:
> > > > > > > -             strscpy(pdev.type, "LnxFIFO");
> > > > > > > +             /* SFU fifo is system file which is empty */
> > > > > > > +             pdev_len = 0;
> > > > > >
> > > > > > My worry about the suggested change above is that it is possible
> > that
> > > > > > we could accidentally match to an empty file.
> > > > >
> > > > > I fully understand your concerns, but code in this patch is for
> > creating
> > > > > new fifos. Not recognizing existing fifos.
> > > > >
> > > > > Code for recognizing existing fifos (=empty file with system
> > attribute)
> > > > > was not changed and is in Linux cifs client for a very long time.
> > > > <>
> > > > > > We intentionally added
> > > > > > the two new dev.type fields for these to avoid collisions with
> > other
> > > > > > things (and since they are Linux specific).  It seems risky to
> > have an
> > > > > > empty file with the system attribute marked as a FIFO, and
> > similarly a
> > > > > > file with one byte null as Socket.   Since this is for only the
> > Linux
> > > > > > client to recognize, I wanted to do something safe for those file
> > > > > > types less likely to be confusing (ie strings for Socket and FIFO
> > that
> > > > > > were similar in length and format to the other special files seemed
> > > > > > intuitive) and "LnxFIFO" and LnxSOCK" were used as the tags in the
> > > > > > file to reduce confusion ie the tags for those two start with
> > "Lnx" -
> > > > > > ie "something used for Linux client" not related to the original
> > > > > > Interix (those begin with "Intx").
> > > > >
> > > > > I see. Now I understand what are those types (as I have not seen them
> > > > > before). It is somehow misleading if such "LnxFIFO" and LnxSOCK"
> > > > > functionality is provided by SFU option, but is incompatible with MS
> > SFU
> > > > > and also with MS NFS server. And is also incompatible with older
> > Linux
> > > > > cifs clients (as they do not understand those Lnx types).
> > > >
> > > > I am not as worried about FIFO and SOCK type being recognized by
> > > > older servers (since almost every use case for them would be for them
> > > > to be seen (only) by the client - e.g. for mounts to servers that
> > > > don't implement reparse points yet), and since they are less
> > > > common file types I am willing to let them be unrecognized by
> > > > old clients (we can tag them for stable if older distros don't have
> > > > them),
> > >
> > > This is quite pity for old clients, to break existing interoperability.
> > > At least I see sfu as an compatibility option either for ecosystem with
> > > old clients, or option where server itself does not support reparse
> > > points.
> > >
> > > > but I am concerned about allowing "current clients" to
> > > > create empty files for an unusual purpose which could be
> > > > confusing/non-intuitive.
> > >
> > > I understand this concern. I thought that this should not be an issue
> > > because files are created with system attribute which is not common for
> > > normal/ordinary usage (system attribute could be less confusing) and
> > > also because this format, at least for fifo is used and understood by
> > > many SW for about 30 years.
> > >
> > > > And since this change (at least the one to allow FIFOs to be created
> > with "sfu"
> > > > has been in mainline for a year and also since it uses a more
> > intuitive tag
> > > > ("LnxFIFO") than the empty one used by very old Windows) the only
> > > > clients who would have created these would be already using this newer
> > tag
> > > > (older Linux clients couldn't have created such files - there seems
> > more
> > > > risk of regression with reverting the change than with continuing with
> > > > the Linux client specific tag (at least to the one for FIFOs
> > > > since that has been in much longer than the socket one which is recent)
> > >
> > > This kind of stuff is lot of times used on LTS/stable linux
> > > distributions and new kernel to these users/admins do not have to be
> > > delivered yet. Mostly it takes 2-3 years after release. Look for example
> > > at RHEL cycles.
> > >
> > > I'm looking on this from opposite perspective. I see this an regression
> > > in -o sfu option that after upgrading from previous LTS version to new,
> > > -o sfu stopped to be compatible with SFU-style fifos.
> > >
> > > But your point is valid. But maybe it is not an issue because users
> > > do not have updated yet to new version?
> > >
> > > > Will discuss with others - opinions welcome.
> > > >
> > > > There is an upcoming SMB3.1.1 test event coming up next week (and the
> > annual
> > > > Storage Developer Conference too) so I can see if others have opinions
> > one
> > > > way or another on whether to move to empty (or 1 byte) files for
> > > > creating fifos/sockets
> > >
> > > Ok, perfect, let me know then about the result.
> > >
> > > > > > Note that in the long run we hope to use reparse points by default
> > in
> > > > > > more servers to store special files like this but there are a few
> > > > > > cases for unusual workloads that need special file support that
> > would
> > > > > > have to use sfu still.  The newer reparse tags that Windows uses
> > "WSL"
> > > > > > have the advantage that they require fewer roundtrips to query
> > (since
> > > > > > the file type is in the reparse tag).
> > > > >
> > > > > Yes, new WSL tags seems to be better. Also SFU mount option is not
> > > > > activated by default.
> > > > >
> > > > > > Also noticed an interesting problem when mounted with "sfu" -
> > > > > > "smbgetinfo filebasicinfo /mnt/fifo1" will hang (in sys_open).  Is
> > > > > > that expected for a FIFO?
> > > > >
> > > > > Reading from fifo sleep reading process until some other process
> > write
> > > > > data to fifo. This is how fifos are working. You can try it on local
> > > > > filesystem (e.g. ext4 or tmpfs).
> > > >
> > > > makes sense - thx
> >

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/4] cifs: Improve client SFU support for special files (2)
  2024-09-12 12:05 ` [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files Pali Rohár
  2024-09-13 20:07   ` Pali Rohár
@ 2024-09-15 19:45   ` Pali Rohár
  2024-09-15 19:45     ` [PATCH 1/4] cifs: Add support for creating SFU symlinks Pali Rohár
                       ` (3 more replies)
  1 sibling, 4 replies; 30+ messages in thread
From: Pali Rohár @ 2024-09-15 19:45 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

Per Steve's request I'm re-sending changes from PATCH 6/7 and
PATCH 7/7 from patch series "cifs: Improve client SFU support for
special files" (Sep 12, 2024). There is no functional change, just split
and reorder.

All changes are:
* Use ARRAY_SIZE() instead of magic value 2
* Patch for SFU symlinks (original 6/7) is the first one
* Patch for SFU fifo/sockets is after symlinks and is split into other
  tree independent patches (socket, fifo and comments), so every one can
  be applied separatetely or dropped if would not be acceptable.

Pali Rohár (4):
  cifs: Add support for creating SFU symlinks
  cifs: Fix creating of SFU socket special files
  cifs: Fix creating of SFU fifo special files
  cifs: Update SFU comments about fifos and sockets

 fs/smb/client/cifspdu.h    |  6 ---
 fs/smb/client/cifsproto.h  |  4 ++
 fs/smb/client/cifssmb.c    |  8 ++--
 fs/smb/client/fs_context.c | 13 ++++---
 fs/smb/client/link.c       |  3 ++
 fs/smb/client/smb1ops.c    |  2 +-
 fs/smb/client/smb2ops.c    | 79 +++++++++++++++++++++++++++++---------
 7 files changed, 80 insertions(+), 35 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/4] cifs: Add support for creating SFU symlinks
  2024-09-15 19:45   ` [PATCH 0/4] cifs: Improve client SFU support for special files (2) Pali Rohár
@ 2024-09-15 19:45     ` Pali Rohár
  2024-09-15 21:15       ` Steve French
  2024-09-27 17:54       ` Enzo Matsumiya
  2024-09-15 19:45     ` [PATCH 2/4] cifs: Fix creating of SFU socket special files Pali Rohár
                       ` (2 subsequent siblings)
  3 siblings, 2 replies; 30+ messages in thread
From: Pali Rohár @ 2024-09-15 19:45 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

Linux cifs client can already detect SFU symlinks and reads it content
(target location). But currently is not able to create new symlink. So
implement this missing support.

When 'sfu' mount option is specified and 'mfsymlinks' is not specified then
create new symlinks in SFU-style. This will provide full SFU compatibility
of symlinks when mounting cifs share with 'sfu' option. 'mfsymlinks' option
override SFU for better Apple compatibility as explained in fs_context.c
file in smb3_update_mnt_flags() function.

Extend __cifs_sfu_make_node() function, which now can handle also S_IFLNK
type and refactor structures passed to sync_write() in this function, by
splitting SFU type and SFU data from original combined struct win_dev as
combined fixed-length struct cannot be used for variable-length symlinks.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifspdu.h    |  6 ---
 fs/smb/client/cifsproto.h  |  4 ++
 fs/smb/client/fs_context.c | 13 ++++---
 fs/smb/client/link.c       |  3 ++
 fs/smb/client/smb2ops.c    | 80 +++++++++++++++++++++++++++++---------
 5 files changed, 77 insertions(+), 29 deletions(-)

diff --git a/fs/smb/client/cifspdu.h b/fs/smb/client/cifspdu.h
index a2072ab9e586..c3b6263060b0 100644
--- a/fs/smb/client/cifspdu.h
+++ b/fs/smb/client/cifspdu.h
@@ -2573,12 +2573,6 @@ typedef struct {
 } __attribute__((packed)) FIND_FILE_STANDARD_INFO; /* level 0x1 FF resp data */
 
 
-struct win_dev {
-	unsigned char type[8]; /* IntxCHR or IntxBLK or LnxFIFO or LnxSOCK */
-	__le64 major;
-	__le64 minor;
-} __attribute__((packed));
-
 struct fea {
 	unsigned char EA_flags;
 	__u8 name_len;
diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index 497bf3c447bc..791bddac0396 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -676,6 +676,10 @@ char *extract_sharename(const char *unc);
 int parse_reparse_point(struct reparse_data_buffer *buf,
 			u32 plen, struct cifs_sb_info *cifs_sb,
 			bool unicode, struct cifs_open_info_data *data);
+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,
+			 const char *symname);
 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);
diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index bc926ab2555b..2f0c3894b0f7 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -1896,14 +1896,17 @@ void smb3_update_mnt_flags(struct cifs_sb_info *cifs_sb)
 	if (ctx->mfsymlinks) {
 		if (ctx->sfu_emul) {
 			/*
-			 * Our SFU ("Services for Unix" emulation does not allow
-			 * creating symlinks but does allow reading existing SFU
-			 * symlinks (it does allow both creating and reading SFU
-			 * style mknod and FIFOs though). When "mfsymlinks" and
+			 * Our SFU ("Services for Unix") emulation allows now
+			 * creating new and reading existing SFU symlinks.
+			 * Older Linux kernel versions were not able to neither
+			 * read existing nor create new SFU symlinks. But
+			 * creating and reading SFU style mknod and FIFOs was
+			 * supported for long time. When "mfsymlinks" and
 			 * "sfu" are both enabled at the same time, it allows
 			 * reading both types of symlinks, but will only create
 			 * them with mfsymlinks format. This allows better
-			 * Apple compatibility (probably better for Samba too)
+			 * Apple compatibility, compatibility with older Linux
+			 * kernel clients (probably better for Samba too)
 			 * while still recognizing old Windows style symlinks.
 			 */
 			cifs_dbg(VFS, "mount options mfsymlinks and sfu both enabled\n");
diff --git a/fs/smb/client/link.c b/fs/smb/client/link.c
index 80099bbb333b..47ddeb7fa111 100644
--- a/fs/smb/client/link.c
+++ b/fs/smb/client/link.c
@@ -606,6 +606,9 @@ cifs_symlink(struct mnt_idmap *idmap, struct inode *inode,
 	/* BB what if DFS and this volume is on different share? BB */
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) {
 		rc = create_mf_symlink(xid, pTcon, cifs_sb, full_path, symname);
+	} else if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) {
+		rc = __cifs_sfu_make_node(xid, inode, direntry, pTcon,
+					  full_path, S_IFLNK, 0, symname);
 #ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
 	} else if (pTcon->unix_ext) {
 		rc = CIFSUnixCreateSymLink(xid, pTcon, full_path, symname,
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 9c2d065d3cc4..2c251e9a3a30 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -5055,9 +5055,10 @@ static int smb2_next_header(struct TCP_Server_Info *server, char *buf,
 	return 0;
 }
 
-static 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)
+				const char *full_path, umode_t mode, dev_t dev,
+				const char *symname)
 {
 	struct TCP_Server_Info *server = tcon->ses->server;
 	struct cifs_open_parms oparms;
@@ -5065,30 +5066,64 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
 	struct cifs_fid fid;
 	unsigned int bytes_written;
-	struct win_dev pdev = {};
-	struct kvec iov[2];
+	u8 type[8];
+	int type_len = 0;
+	struct {
+		__le64 major;
+		__le64 minor;
+	} __packed pdev = {};
+	__le16 *symname_utf16 = NULL;
+	u8 *data = NULL;
+	int data_len = 0;
+	struct kvec iov[3];
 	__u32 oplock = server->oplocks ? REQ_OPLOCK : 0;
 	int rc;
 
 	switch (mode & S_IFMT) {
 	case S_IFCHR:
-		memcpy(pdev.type, "IntxCHR\0", 8);
+		type_len = 8;
+		memcpy(type, "IntxCHR\0", type_len);
 		pdev.major = cpu_to_le64(MAJOR(dev));
 		pdev.minor = cpu_to_le64(MINOR(dev));
+		data = (u8 *)&pdev;
+		data_len = sizeof(pdev);
 		break;
 	case S_IFBLK:
-		memcpy(pdev.type, "IntxBLK\0", 8);
+		type_len = 8;
+		memcpy(type, "IntxBLK\0", type_len);
 		pdev.major = cpu_to_le64(MAJOR(dev));
 		pdev.minor = cpu_to_le64(MINOR(dev));
+		data = (u8 *)&pdev;
+		data_len = sizeof(pdev);
+		break;
+	case S_IFLNK:
+		type_len = 8;
+		memcpy(type, "IntxLNK\1", type_len);
+		symname_utf16 = cifs_strndup_to_utf16(symname, strlen(symname),
+						      &data_len, cifs_sb->local_nls,
+						      NO_MAP_UNI_RSVD);
+		if (!symname_utf16) {
+			rc = -ENOMEM;
+			goto out;
+		}
+		data_len -= 2; /* symlink is without trailing wide-nul */
+		data = (u8 *)symname_utf16;
 		break;
 	case S_IFSOCK:
-		strscpy(pdev.type, "LnxSOCK");
+		type_len = 8;
+		strscpy(type, "LnxSOCK");
+		data = (u8 *)&pdev;
+		data_len = sizeof(pdev);
 		break;
 	case S_IFIFO:
-		strscpy(pdev.type, "LnxFIFO");
+		type_len = 8;
+		strscpy(type, "LnxFIFO");
+		data = (u8 *)&pdev;
+		data_len = sizeof(pdev);
 		break;
 	default:
-		return -EPERM;
+		rc = -EPERM;
+		goto out;
 	}
 
 	oparms = CIFS_OPARMS(cifs_sb, tcon, full_path, GENERIC_WRITE,
@@ -5098,17 +5133,26 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 
 	rc = server->ops->open(xid, &oparms, &oplock, NULL);
 	if (rc)
-		return rc;
+		goto out;
 
-	io_parms.pid = current->tgid;
-	io_parms.tcon = tcon;
-	io_parms.length = sizeof(pdev);
-	iov[1].iov_base = &pdev;
-	iov[1].iov_len = sizeof(pdev);
+	if (type_len + data_len > 0) {
+		io_parms.pid = current->tgid;
+		io_parms.tcon = tcon;
+		io_parms.length = type_len + data_len;
+		iov[1].iov_base = type;
+		iov[1].iov_len = type_len;
+		iov[2].iov_base = data;
+		iov[2].iov_len = data_len;
+
+		rc = server->ops->sync_write(xid, &fid, &io_parms,
+					     &bytes_written,
+					     iov, ARRAY_SIZE(iov)-1);
+	}
 
-	rc = server->ops->sync_write(xid, &fid, &io_parms,
-				     &bytes_written, iov, 1);
 	server->ops->close(xid, tcon, &fid);
+
+out:
+	kfree(symname_utf16);
 	return rc;
 }
 
@@ -5120,7 +5164,7 @@ int cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 	int rc;
 
 	rc = __cifs_sfu_make_node(xid, inode, dentry, tcon,
-				  full_path, mode, dev);
+				  full_path, mode, dev, NULL);
 	if (rc)
 		return rc;
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/4] cifs: Fix creating of SFU socket special files
  2024-09-15 19:45   ` [PATCH 0/4] cifs: Improve client SFU support for special files (2) Pali Rohár
  2024-09-15 19:45     ` [PATCH 1/4] cifs: Add support for creating SFU symlinks Pali Rohár
@ 2024-09-15 19:45     ` Pali Rohár
  2024-09-15 19:45     ` [PATCH 3/4] cifs: Fix creating of SFU fifo " Pali Rohár
  2024-09-15 19:45     ` [PATCH 4/4] cifs: Update SFU comments about fifos and sockets Pali Rohár
  3 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2024-09-15 19:45 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

SFU-style socket is file which has system attribute set and file content is
one zero byte. This format was introduced in Interix 3.0 subsystem, as part
of the Microsoft SFU 3.0 and is used also by all later versions. Previous
versions had no UNIX domain socket support.

This format of SFU-style sockets is recognized also by Windows NFS server
included in the latest version on Windows Server 2022.

Currently when sfu mount option is specified then CIFS creates new socket
files with content LnxSOCK. This was introduced in commit 518549c120e6
("cifs: fix creating sockets when using sfu mount options") as nobody
figured out what is the correct SFU format of sockets and tag LnxSOCK was
chosen to allow creating socket files. LnxSOCK looks similar to IntxCHR and
IntxBLK tags which are the proper SFU tags for char and block devices.

It is important to note that LnxSOCK is not SFU-compatible and neither
Interix, SFU, SUA or Windows NFS server recognize file with content of
LnxSOCK as special socket file.

Now when the proper format of SFU-style sockets is known and it was
verified that works with both old Interix 3.x subsystem and also with
Windows NFS server, change implementation of creating new SFU socket files
by CIFS client to be compatible with SFU.

518549c120e6 ("cifs: fix creating sockets when using sfu mount options")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/smb2ops.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 2c251e9a3a30..dc56f7ba1a06 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -5110,10 +5110,9 @@ int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 		data = (u8 *)symname_utf16;
 		break;
 	case S_IFSOCK:
-		type_len = 8;
-		strscpy(type, "LnxSOCK");
-		data = (u8 *)&pdev;
-		data_len = sizeof(pdev);
+		/* SFU socket is system file with one zero byte */
+		type_len = 1;
+		type[0] = '\0';
 		break;
 	case S_IFIFO:
 		type_len = 8;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/4] cifs: Fix creating of SFU fifo special files
  2024-09-15 19:45   ` [PATCH 0/4] cifs: Improve client SFU support for special files (2) Pali Rohár
  2024-09-15 19:45     ` [PATCH 1/4] cifs: Add support for creating SFU symlinks Pali Rohár
  2024-09-15 19:45     ` [PATCH 2/4] cifs: Fix creating of SFU socket special files Pali Rohár
@ 2024-09-15 19:45     ` Pali Rohár
  2024-09-15 19:45     ` [PATCH 4/4] cifs: Update SFU comments about fifos and sockets Pali Rohár
  3 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2024-09-15 19:45 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

SFU-style fifo is empty file with system attribute set. This format is used
by old Microsoft POSIX subsystem and later also by OpenNT/Interix subsystem
(which replaced Microsoft POSIX subsystem and is part of Microsoft SFU).

This format of SFU-style fifos is recognized also by Windows NFS server
included in the latest version on Windows Server 2022.

Currently when sfu mount option is specified then CIFS creates new fifo
files with content LnxFIFO. This was introduced in commit 72bc63f5e23a
("smb3: fix creating FIFOs when mounting with "sfu" mount option").

It is important to note that LnxFIFO is not SFU-compatible and neither
Interix, SFU, SUA or Windows NFS server recognize file with content of
LnxFIFO as special fifo file.

So when sfu mount option is specified, fix this problem and create all fifo
special files compatible with SFU format, as it is expected by sfu mount
option. This allows interoperability with other SFU implementations and
also with Windows NFS server.

Fixes: 72bc63f5e23a ("smb3: fix creating FIFOs when mounting with "sfu" mount option")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/smb2ops.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index dc56f7ba1a06..406f2399f0c5 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -5115,10 +5115,8 @@ int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
 		type[0] = '\0';
 		break;
 	case S_IFIFO:
-		type_len = 8;
-		strscpy(type, "LnxFIFO");
-		data = (u8 *)&pdev;
-		data_len = sizeof(pdev);
+		/* SFU fifo is system file which is empty */
+		type_len = 0;
 		break;
 	default:
 		rc = -EPERM;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 4/4] cifs: Update SFU comments about fifos and sockets
  2024-09-15 19:45   ` [PATCH 0/4] cifs: Improve client SFU support for special files (2) Pali Rohár
                       ` (2 preceding siblings ...)
  2024-09-15 19:45     ` [PATCH 3/4] cifs: Fix creating of SFU fifo " Pali Rohár
@ 2024-09-15 19:45     ` Pali Rohár
  2024-09-15 21:14       ` Steve French
  3 siblings, 1 reply; 30+ messages in thread
From: Pali Rohár @ 2024-09-15 19:45 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg; +Cc: linux-cifs, linux-kernel

In SFU mode, activated by -o sfu mount option is now also support for
creating new fifos and sockets.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifssmb.c | 8 ++++----
 fs/smb/client/smb1ops.c | 2 +-
 fs/smb/client/smb2ops.c | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index cfae2e918209..0ffc45aa5e2c 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -1076,8 +1076,8 @@ SMBLegacyOpen(const unsigned int xid, struct cifs_tcon *tcon,
 	pSMB->OpenFlags |= cpu_to_le16(REQ_MORE_INFO);
 	pSMB->Mode = cpu_to_le16(access_flags_to_smbopen_mode(access_flags));
 	pSMB->Mode |= cpu_to_le16(0x40); /* deny none */
-	/* set file as system file if special file such
-	   as fifo and server expecting SFU style and
+	/* set file as system file if special file such as fifo,
+	 * socket, char or block and server expecting SFU style and
 	   no Unix extensions */
 
 	if (create_options & CREATE_OPTION_SPECIAL)
@@ -1193,8 +1193,8 @@ CIFS_open(const unsigned int xid, struct cifs_open_parms *oparms, int *oplock,
 	req->AllocationSize = 0;
 
 	/*
-	 * Set file as system file if special file such as fifo and server
-	 * expecting SFU style and no Unix extensions.
+	 * Set file as system file if special file such as fifo, socket, char
+	 * or block and server expecting SFU style and no Unix extensions.
 	 */
 	if (create_options & CREATE_OPTION_SPECIAL)
 		req->FileAttributes = cpu_to_le32(ATTR_SYSTEM);
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index e1f2feb56f45..e03c91a49650 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -1078,7 +1078,7 @@ cifs_make_node(unsigned int xid, struct inode *inode,
 	/*
 	 * Check if mounted with mount parm 'sfu' mount parm.
 	 * SFU emulation should work with all servers, but only
-	 * supports block and char device (no socket & fifo),
+	 * supports block and char device, socket & fifo,
 	 * and was used by default in earlier versions of Windows
 	 */
 	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL))
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 406f2399f0c5..d30f7cab197e 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -5190,7 +5190,7 @@ static int smb2_make_node(unsigned int xid, struct inode *inode,
 	/*
 	 * Check if mounted with mount parm 'sfu' mount parm.
 	 * SFU emulation should work with all servers, but only
-	 * supports block and char device (no socket & fifo),
+	 * supports block and char device, socket & fifo,
 	 * and was used by default in earlier versions of Windows
 	 */
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH 4/4] cifs: Update SFU comments about fifos and sockets
  2024-09-15 19:45     ` [PATCH 4/4] cifs: Update SFU comments about fifos and sockets Pali Rohár
@ 2024-09-15 21:14       ` Steve French
  0 siblings, 0 replies; 30+ messages in thread
From: Steve French @ 2024-09-15 21:14 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, linux-cifs,
	linux-kernel

merged into cifs-2.6.git for-next

On Sun, Sep 15, 2024 at 2:46 PM Pali Rohár <pali@kernel.org> wrote:
>
> In SFU mode, activated by -o sfu mount option is now also support for
> creating new fifos and sockets.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  fs/smb/client/cifssmb.c | 8 ++++----
>  fs/smb/client/smb1ops.c | 2 +-
>  fs/smb/client/smb2ops.c | 2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
> index cfae2e918209..0ffc45aa5e2c 100644
> --- a/fs/smb/client/cifssmb.c
> +++ b/fs/smb/client/cifssmb.c
> @@ -1076,8 +1076,8 @@ SMBLegacyOpen(const unsigned int xid, struct cifs_tcon *tcon,
>         pSMB->OpenFlags |= cpu_to_le16(REQ_MORE_INFO);
>         pSMB->Mode = cpu_to_le16(access_flags_to_smbopen_mode(access_flags));
>         pSMB->Mode |= cpu_to_le16(0x40); /* deny none */
> -       /* set file as system file if special file such
> -          as fifo and server expecting SFU style and
> +       /* set file as system file if special file such as fifo,
> +        * socket, char or block and server expecting SFU style and
>            no Unix extensions */
>
>         if (create_options & CREATE_OPTION_SPECIAL)
> @@ -1193,8 +1193,8 @@ CIFS_open(const unsigned int xid, struct cifs_open_parms *oparms, int *oplock,
>         req->AllocationSize = 0;
>
>         /*
> -        * Set file as system file if special file such as fifo and server
> -        * expecting SFU style and no Unix extensions.
> +        * Set file as system file if special file such as fifo, socket, char
> +        * or block and server expecting SFU style and no Unix extensions.
>          */
>         if (create_options & CREATE_OPTION_SPECIAL)
>                 req->FileAttributes = cpu_to_le32(ATTR_SYSTEM);
> diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
> index e1f2feb56f45..e03c91a49650 100644
> --- a/fs/smb/client/smb1ops.c
> +++ b/fs/smb/client/smb1ops.c
> @@ -1078,7 +1078,7 @@ cifs_make_node(unsigned int xid, struct inode *inode,
>         /*
>          * Check if mounted with mount parm 'sfu' mount parm.
>          * SFU emulation should work with all servers, but only
> -        * supports block and char device (no socket & fifo),
> +        * supports block and char device, socket & fifo,
>          * and was used by default in earlier versions of Windows
>          */
>         if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL))
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> index 406f2399f0c5..d30f7cab197e 100644
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -5190,7 +5190,7 @@ static int smb2_make_node(unsigned int xid, struct inode *inode,
>         /*
>          * Check if mounted with mount parm 'sfu' mount parm.
>          * SFU emulation should work with all servers, but only
> -        * supports block and char device (no socket & fifo),
> +        * supports block and char device, socket & fifo,
>          * and was used by default in earlier versions of Windows
>          */
>         if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) {
> --
> 2.20.1
>
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/4] cifs: Add support for creating SFU symlinks
  2024-09-15 19:45     ` [PATCH 1/4] cifs: Add support for creating SFU symlinks Pali Rohár
@ 2024-09-15 21:15       ` Steve French
  2024-09-27 17:54       ` Enzo Matsumiya
  1 sibling, 0 replies; 30+ messages in thread
From: Steve French @ 2024-09-15 21:15 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, linux-cifs,
	linux-kernel

merged into cifs-2.6.git for-next pending review/testing

On Sun, Sep 15, 2024 at 2:46 PM Pali Rohár <pali@kernel.org> wrote:
>
> Linux cifs client can already detect SFU symlinks and reads it content
> (target location). But currently is not able to create new symlink. So
> implement this missing support.
>
> When 'sfu' mount option is specified and 'mfsymlinks' is not specified then
> create new symlinks in SFU-style. This will provide full SFU compatibility
> of symlinks when mounting cifs share with 'sfu' option. 'mfsymlinks' option
> override SFU for better Apple compatibility as explained in fs_context.c
> file in smb3_update_mnt_flags() function.
>
> Extend __cifs_sfu_make_node() function, which now can handle also S_IFLNK
> type and refactor structures passed to sync_write() in this function, by
> splitting SFU type and SFU data from original combined struct win_dev as
> combined fixed-length struct cannot be used for variable-length symlinks.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  fs/smb/client/cifspdu.h    |  6 ---
>  fs/smb/client/cifsproto.h  |  4 ++
>  fs/smb/client/fs_context.c | 13 ++++---
>  fs/smb/client/link.c       |  3 ++
>  fs/smb/client/smb2ops.c    | 80 +++++++++++++++++++++++++++++---------
>  5 files changed, 77 insertions(+), 29 deletions(-)
>
> diff --git a/fs/smb/client/cifspdu.h b/fs/smb/client/cifspdu.h
> index a2072ab9e586..c3b6263060b0 100644
> --- a/fs/smb/client/cifspdu.h
> +++ b/fs/smb/client/cifspdu.h
> @@ -2573,12 +2573,6 @@ typedef struct {
>  } __attribute__((packed)) FIND_FILE_STANDARD_INFO; /* level 0x1 FF resp data */
>
>
> -struct win_dev {
> -       unsigned char type[8]; /* IntxCHR or IntxBLK or LnxFIFO or LnxSOCK */
> -       __le64 major;
> -       __le64 minor;
> -} __attribute__((packed));
> -
>  struct fea {
>         unsigned char EA_flags;
>         __u8 name_len;
> diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
> index 497bf3c447bc..791bddac0396 100644
> --- a/fs/smb/client/cifsproto.h
> +++ b/fs/smb/client/cifsproto.h
> @@ -676,6 +676,10 @@ char *extract_sharename(const char *unc);
>  int parse_reparse_point(struct reparse_data_buffer *buf,
>                         u32 plen, struct cifs_sb_info *cifs_sb,
>                         bool unicode, struct cifs_open_info_data *data);
> +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,
> +                        const char *symname);
>  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);
> diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
> index bc926ab2555b..2f0c3894b0f7 100644
> --- a/fs/smb/client/fs_context.c
> +++ b/fs/smb/client/fs_context.c
> @@ -1896,14 +1896,17 @@ void smb3_update_mnt_flags(struct cifs_sb_info *cifs_sb)
>         if (ctx->mfsymlinks) {
>                 if (ctx->sfu_emul) {
>                         /*
> -                        * Our SFU ("Services for Unix" emulation does not allow
> -                        * creating symlinks but does allow reading existing SFU
> -                        * symlinks (it does allow both creating and reading SFU
> -                        * style mknod and FIFOs though). When "mfsymlinks" and
> +                        * Our SFU ("Services for Unix") emulation allows now
> +                        * creating new and reading existing SFU symlinks.
> +                        * Older Linux kernel versions were not able to neither
> +                        * read existing nor create new SFU symlinks. But
> +                        * creating and reading SFU style mknod and FIFOs was
> +                        * supported for long time. When "mfsymlinks" and
>                          * "sfu" are both enabled at the same time, it allows
>                          * reading both types of symlinks, but will only create
>                          * them with mfsymlinks format. This allows better
> -                        * Apple compatibility (probably better for Samba too)
> +                        * Apple compatibility, compatibility with older Linux
> +                        * kernel clients (probably better for Samba too)
>                          * while still recognizing old Windows style symlinks.
>                          */
>                         cifs_dbg(VFS, "mount options mfsymlinks and sfu both enabled\n");
> diff --git a/fs/smb/client/link.c b/fs/smb/client/link.c
> index 80099bbb333b..47ddeb7fa111 100644
> --- a/fs/smb/client/link.c
> +++ b/fs/smb/client/link.c
> @@ -606,6 +606,9 @@ cifs_symlink(struct mnt_idmap *idmap, struct inode *inode,
>         /* BB what if DFS and this volume is on different share? BB */
>         if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) {
>                 rc = create_mf_symlink(xid, pTcon, cifs_sb, full_path, symname);
> +       } else if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) {
> +               rc = __cifs_sfu_make_node(xid, inode, direntry, pTcon,
> +                                         full_path, S_IFLNK, 0, symname);
>  #ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
>         } else if (pTcon->unix_ext) {
>                 rc = CIFSUnixCreateSymLink(xid, pTcon, full_path, symname,
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> index 9c2d065d3cc4..2c251e9a3a30 100644
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -5055,9 +5055,10 @@ static int smb2_next_header(struct TCP_Server_Info *server, char *buf,
>         return 0;
>  }
>
> -static 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)
> +                               const char *full_path, umode_t mode, dev_t dev,
> +                               const char *symname)
>  {
>         struct TCP_Server_Info *server = tcon->ses->server;
>         struct cifs_open_parms oparms;
> @@ -5065,30 +5066,64 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
>         struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
>         struct cifs_fid fid;
>         unsigned int bytes_written;
> -       struct win_dev pdev = {};
> -       struct kvec iov[2];
> +       u8 type[8];
> +       int type_len = 0;
> +       struct {
> +               __le64 major;
> +               __le64 minor;
> +       } __packed pdev = {};
> +       __le16 *symname_utf16 = NULL;
> +       u8 *data = NULL;
> +       int data_len = 0;
> +       struct kvec iov[3];
>         __u32 oplock = server->oplocks ? REQ_OPLOCK : 0;
>         int rc;
>
>         switch (mode & S_IFMT) {
>         case S_IFCHR:
> -               memcpy(pdev.type, "IntxCHR\0", 8);
> +               type_len = 8;
> +               memcpy(type, "IntxCHR\0", type_len);
>                 pdev.major = cpu_to_le64(MAJOR(dev));
>                 pdev.minor = cpu_to_le64(MINOR(dev));
> +               data = (u8 *)&pdev;
> +               data_len = sizeof(pdev);
>                 break;
>         case S_IFBLK:
> -               memcpy(pdev.type, "IntxBLK\0", 8);
> +               type_len = 8;
> +               memcpy(type, "IntxBLK\0", type_len);
>                 pdev.major = cpu_to_le64(MAJOR(dev));
>                 pdev.minor = cpu_to_le64(MINOR(dev));
> +               data = (u8 *)&pdev;
> +               data_len = sizeof(pdev);
> +               break;
> +       case S_IFLNK:
> +               type_len = 8;
> +               memcpy(type, "IntxLNK\1", type_len);
> +               symname_utf16 = cifs_strndup_to_utf16(symname, strlen(symname),
> +                                                     &data_len, cifs_sb->local_nls,
> +                                                     NO_MAP_UNI_RSVD);
> +               if (!symname_utf16) {
> +                       rc = -ENOMEM;
> +                       goto out;
> +               }
> +               data_len -= 2; /* symlink is without trailing wide-nul */
> +               data = (u8 *)symname_utf16;
>                 break;
>         case S_IFSOCK:
> -               strscpy(pdev.type, "LnxSOCK");
> +               type_len = 8;
> +               strscpy(type, "LnxSOCK");
> +               data = (u8 *)&pdev;
> +               data_len = sizeof(pdev);
>                 break;
>         case S_IFIFO:
> -               strscpy(pdev.type, "LnxFIFO");
> +               type_len = 8;
> +               strscpy(type, "LnxFIFO");
> +               data = (u8 *)&pdev;
> +               data_len = sizeof(pdev);
>                 break;
>         default:
> -               return -EPERM;
> +               rc = -EPERM;
> +               goto out;
>         }
>
>         oparms = CIFS_OPARMS(cifs_sb, tcon, full_path, GENERIC_WRITE,
> @@ -5098,17 +5133,26 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
>
>         rc = server->ops->open(xid, &oparms, &oplock, NULL);
>         if (rc)
> -               return rc;
> +               goto out;
>
> -       io_parms.pid = current->tgid;
> -       io_parms.tcon = tcon;
> -       io_parms.length = sizeof(pdev);
> -       iov[1].iov_base = &pdev;
> -       iov[1].iov_len = sizeof(pdev);
> +       if (type_len + data_len > 0) {
> +               io_parms.pid = current->tgid;
> +               io_parms.tcon = tcon;
> +               io_parms.length = type_len + data_len;
> +               iov[1].iov_base = type;
> +               iov[1].iov_len = type_len;
> +               iov[2].iov_base = data;
> +               iov[2].iov_len = data_len;
> +
> +               rc = server->ops->sync_write(xid, &fid, &io_parms,
> +                                            &bytes_written,
> +                                            iov, ARRAY_SIZE(iov)-1);
> +       }
>
> -       rc = server->ops->sync_write(xid, &fid, &io_parms,
> -                                    &bytes_written, iov, 1);
>         server->ops->close(xid, tcon, &fid);
> +
> +out:
> +       kfree(symname_utf16);
>         return rc;
>  }
>
> @@ -5120,7 +5164,7 @@ int cifs_sfu_make_node(unsigned int xid, struct inode *inode,
>         int rc;
>
>         rc = __cifs_sfu_make_node(xid, inode, dentry, tcon,
> -                                 full_path, mode, dev);
> +                                 full_path, mode, dev, NULL);
>         if (rc)
>                 return rc;
>
> --
> 2.20.1
>
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files
  2024-09-15 17:48                 ` Pali Rohár
@ 2024-09-16 16:23                   ` Pali Rohár
  0 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2024-09-16 16:23 UTC (permalink / raw)
  To: Steve French; +Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, CIFS, LKML

Now I have tested this behavior against one Windows Server 2019 instance
and it works too. So confirmed, it is the case also for 2019 version.

On Sunday 15 September 2024 19:48:24 Pali Rohár wrote:
> It works for this new 2022 version and also for old 2012 version. So my
> personal guess is yes, that it would work also for 2019 (I do not see
> reason why something would not work for version in-the-middle). If
> needed, I may try to do this test against 2019 version during next or
> another weekend.
> 
> On Sunday 15 September 2024 12:42:27 Steve French wrote:
> > Do you know if this is the case for windows server 2019
> > 
> > On Sun, Sep 15, 2024, 12:41 PM Pali Rohár <pali@kernel.org> wrote:
> > 
> > > Hello Steve,
> > >
> > > I have another argument for the empty system file as fifo over the
> > > file with "LnxFIFO" content.
> > >
> > > Now I have figured out that even the latest Windows Server 2022 version
> > > provides interoperability of FIFOs in SFU format with Windows NFS 4.1
> > > Server. So if you configure on Windows Server 2022 one share which is
> > > exported over SMB and also NFS at the same time, and over SMB you create
> > > SFU-style fifo, then Windows NFS4.1 server recognize it and properly
> > > reports nfs4type as NFS4FIFO for NFSv4.1 client.
> > >
> > > So this SFU FIFO style is not only for old clients and servers, but
> > > still relevant and useful even for latest Windows Server.
> > >
> > > And same applies for named sockets.
> > >
> > > For testing this scenario it is enough to use just trial version of
> > > latest Windows Server from:
> > > https://go.microsoft.com/fwlink/p/?linkid=2208182
> > > https://go.microsoft.com/fwlink/p/?Linkid=2195280
> > >
> > > For setting NFS4.1 and SMB share named "test" I used these cmd commands:
> > >
> > >   dism /Online /Enable-Feature /All
> > > /FeatureName:ServerForNFS-Infrastructure
> > >   md C:\test
> > >   icacls C:\test /grant Everyone:(OI)(CI)F /T
> > >   nfsshare test=C:\test -o rw unmapped=yes
> > >   net share test=C:\test /grant:Everyone,FULL
> > >
> > > (for everyone who is going to reproduce this scenario, beware that new
> > > Windows servers use by default powershell, so first launch cmd.exe then
> > > copy+paste those commands)
> > >
> > > Pali
> > >
> > > On Saturday 14 September 2024 10:17:42 Pali Rohár wrote:
> > > > On Saturday 14 September 2024 01:21:17 Steve French wrote:
> > > > > On Fri, Sep 13, 2024 at 5:42 PM Pali Rohár <pali@kernel.org> wrote:
> > > > > >
> > > > > > On Friday 13 September 2024 17:14:22 Steve French wrote:
> > > > > > > How did you find the format of the FIFO and SOCK file types?  I
> > > > > >
> > > > > > For fifo there are multiple sources on internet, but none of them is
> > > > > > normative. Everything is just what people have tried. For example
> > > this
> > > > > > old email on samba list:
> > > > > >
> > > https://lists.samba.org/archive/linux-cifs-client/2005-May/000856.html
> > > > > >
> > > > > > Format of the socket I have figured out by creating it in Interix
> > > > > > subsystem and then dumped content of the file from Win32 subsystem.
> > > > > > Then I checked that it has also same format over older MS NFS server.
> > > > > > It was easier than trying to search for some documentation (which I
> > > have
> > > > > > not found).
> > > > > >
> > > > > > > couldn't find any references to those so added two new types to
> > > allow
> > > > > > > current Linux to be able to create these (especially since they are
> > > > > > > opaque to the server and thus low risk).
> > > > > >
> > > > > > I was searching over internet again and now I have found patent
> > > > > > https://patents.google.com/patent/US20090049459 which describe
> > > symlink
> > > > > > content:
> > > > > >
> > > > > > #define NFS_SPECFILE_LNK_V1 0x014b4e4c78746e49 /* “IntxLNK” */
> > > > > >
> > > > > > But does not describe other types.
> > > > > >
> > > > > > > > +     case S_IFSOCK:
> > > > > > > > -             strscpy(pdev.type, "LnxSOCK");
> > > > > > > > +             /* SFU socket is system file with one zero byte */
> > > > > > > > +             pdev_len = 1;
> > > > > > > > +             pdev.type[0] = '\0';
> > > > > > > >               break;
> > > > > > > >       case S_IFIFO:
> > > > > > > > -             strscpy(pdev.type, "LnxFIFO");
> > > > > > > > +             /* SFU fifo is system file which is empty */
> > > > > > > > +             pdev_len = 0;
> > > > > > >
> > > > > > > My worry about the suggested change above is that it is possible
> > > that
> > > > > > > we could accidentally match to an empty file.
> > > > > >
> > > > > > I fully understand your concerns, but code in this patch is for
> > > creating
> > > > > > new fifos. Not recognizing existing fifos.
> > > > > >
> > > > > > Code for recognizing existing fifos (=empty file with system
> > > attribute)
> > > > > > was not changed and is in Linux cifs client for a very long time.
> > > > > <>
> > > > > > > We intentionally added
> > > > > > > the two new dev.type fields for these to avoid collisions with
> > > other
> > > > > > > things (and since they are Linux specific).  It seems risky to
> > > have an
> > > > > > > empty file with the system attribute marked as a FIFO, and
> > > similarly a
> > > > > > > file with one byte null as Socket.   Since this is for only the
> > > Linux
> > > > > > > client to recognize, I wanted to do something safe for those file
> > > > > > > types less likely to be confusing (ie strings for Socket and FIFO
> > > that
> > > > > > > were similar in length and format to the other special files seemed
> > > > > > > intuitive) and "LnxFIFO" and LnxSOCK" were used as the tags in the
> > > > > > > file to reduce confusion ie the tags for those two start with
> > > "Lnx" -
> > > > > > > ie "something used for Linux client" not related to the original
> > > > > > > Interix (those begin with "Intx").
> > > > > >
> > > > > > I see. Now I understand what are those types (as I have not seen them
> > > > > > before). It is somehow misleading if such "LnxFIFO" and LnxSOCK"
> > > > > > functionality is provided by SFU option, but is incompatible with MS
> > > SFU
> > > > > > and also with MS NFS server. And is also incompatible with older
> > > Linux
> > > > > > cifs clients (as they do not understand those Lnx types).
> > > > >
> > > > > I am not as worried about FIFO and SOCK type being recognized by
> > > > > older servers (since almost every use case for them would be for them
> > > > > to be seen (only) by the client - e.g. for mounts to servers that
> > > > > don't implement reparse points yet), and since they are less
> > > > > common file types I am willing to let them be unrecognized by
> > > > > old clients (we can tag them for stable if older distros don't have
> > > > > them),
> > > >
> > > > This is quite pity for old clients, to break existing interoperability.
> > > > At least I see sfu as an compatibility option either for ecosystem with
> > > > old clients, or option where server itself does not support reparse
> > > > points.
> > > >
> > > > > but I am concerned about allowing "current clients" to
> > > > > create empty files for an unusual purpose which could be
> > > > > confusing/non-intuitive.
> > > >
> > > > I understand this concern. I thought that this should not be an issue
> > > > because files are created with system attribute which is not common for
> > > > normal/ordinary usage (system attribute could be less confusing) and
> > > > also because this format, at least for fifo is used and understood by
> > > > many SW for about 30 years.
> > > >
> > > > > And since this change (at least the one to allow FIFOs to be created
> > > with "sfu"
> > > > > has been in mainline for a year and also since it uses a more
> > > intuitive tag
> > > > > ("LnxFIFO") than the empty one used by very old Windows) the only
> > > > > clients who would have created these would be already using this newer
> > > tag
> > > > > (older Linux clients couldn't have created such files - there seems
> > > more
> > > > > risk of regression with reverting the change than with continuing with
> > > > > the Linux client specific tag (at least to the one for FIFOs
> > > > > since that has been in much longer than the socket one which is recent)
> > > >
> > > > This kind of stuff is lot of times used on LTS/stable linux
> > > > distributions and new kernel to these users/admins do not have to be
> > > > delivered yet. Mostly it takes 2-3 years after release. Look for example
> > > > at RHEL cycles.
> > > >
> > > > I'm looking on this from opposite perspective. I see this an regression
> > > > in -o sfu option that after upgrading from previous LTS version to new,
> > > > -o sfu stopped to be compatible with SFU-style fifos.
> > > >
> > > > But your point is valid. But maybe it is not an issue because users
> > > > do not have updated yet to new version?
> > > >
> > > > > Will discuss with others - opinions welcome.
> > > > >
> > > > > There is an upcoming SMB3.1.1 test event coming up next week (and the
> > > annual
> > > > > Storage Developer Conference too) so I can see if others have opinions
> > > one
> > > > > way or another on whether to move to empty (or 1 byte) files for
> > > > > creating fifos/sockets
> > > >
> > > > Ok, perfect, let me know then about the result.
> > > >
> > > > > > > Note that in the long run we hope to use reparse points by default
> > > in
> > > > > > > more servers to store special files like this but there are a few
> > > > > > > cases for unusual workloads that need special file support that
> > > would
> > > > > > > have to use sfu still.  The newer reparse tags that Windows uses
> > > "WSL"
> > > > > > > have the advantage that they require fewer roundtrips to query
> > > (since
> > > > > > > the file type is in the reparse tag).
> > > > > >
> > > > > > Yes, new WSL tags seems to be better. Also SFU mount option is not
> > > > > > activated by default.
> > > > > >
> > > > > > > Also noticed an interesting problem when mounted with "sfu" -
> > > > > > > "smbgetinfo filebasicinfo /mnt/fifo1" will hang (in sys_open).  Is
> > > > > > > that expected for a FIFO?
> > > > > >
> > > > > > Reading from fifo sleep reading process until some other process
> > > write
> > > > > > data to fifo. This is how fifos are working. You can try it on local
> > > > > > filesystem (e.g. ext4 or tmpfs).
> > > > >
> > > > > makes sense - thx
> > >

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/4] cifs: Add support for creating SFU symlinks
  2024-09-15 19:45     ` [PATCH 1/4] cifs: Add support for creating SFU symlinks Pali Rohár
  2024-09-15 21:15       ` Steve French
@ 2024-09-27 17:54       ` Enzo Matsumiya
  2024-09-27 18:11         ` Pali Rohár
  1 sibling, 1 reply; 30+ messages in thread
From: Enzo Matsumiya @ 2024-09-27 17:54 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, linux-cifs,
	linux-kernel

Hi Pali,

On 09/15, Pali Rohár wrote:
>Linux cifs client can already detect SFU symlinks and reads it content
>(target location). But currently is not able to create new symlink. So
>implement this missing support.
>
>When 'sfu' mount option is specified and 'mfsymlinks' is not specified then
>create new symlinks in SFU-style. This will provide full SFU compatibility
>of symlinks when mounting cifs share with 'sfu' option. 'mfsymlinks' option
>override SFU for better Apple compatibility as explained in fs_context.c
>file in smb3_update_mnt_flags() function.
>
>Extend __cifs_sfu_make_node() function, which now can handle also S_IFLNK
>type and refactor structures passed to sync_write() in this function, by
>splitting SFU type and SFU data from original combined struct win_dev as
>combined fixed-length struct cannot be used for variable-length symlinks.
>
>Signed-off-by: Pali Rohár <pali@kernel.org>
>---
> fs/smb/client/cifspdu.h    |  6 ---
> fs/smb/client/cifsproto.h  |  4 ++
> fs/smb/client/fs_context.c | 13 ++++---
> fs/smb/client/link.c       |  3 ++
> fs/smb/client/smb2ops.c    | 80 +++++++++++++++++++++++++++++---------
> 5 files changed, 77 insertions(+), 29 deletions(-)
>
>diff --git a/fs/smb/client/cifspdu.h b/fs/smb/client/cifspdu.h
>index a2072ab9e586..c3b6263060b0 100644
>--- a/fs/smb/client/cifspdu.h
>+++ b/fs/smb/client/cifspdu.h
>@@ -2573,12 +2573,6 @@ typedef struct {
> } __attribute__((packed)) FIND_FILE_STANDARD_INFO; /* level 0x1 FF resp data */
>
>
>-struct win_dev {
>-	unsigned char type[8]; /* IntxCHR or IntxBLK or LnxFIFO or LnxSOCK */
>-	__le64 major;
>-	__le64 minor;
>-} __attribute__((packed));
>-
> struct fea {
> 	unsigned char EA_flags;
> 	__u8 name_len;
>diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
>index 497bf3c447bc..791bddac0396 100644
>--- a/fs/smb/client/cifsproto.h
>+++ b/fs/smb/client/cifsproto.h
>@@ -676,6 +676,10 @@ char *extract_sharename(const char *unc);
> int parse_reparse_point(struct reparse_data_buffer *buf,
> 			u32 plen, struct cifs_sb_info *cifs_sb,
> 			bool unicode, struct cifs_open_info_data *data);
>+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,
>+			 const char *symname);
> 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);
>diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
>index bc926ab2555b..2f0c3894b0f7 100644
>--- a/fs/smb/client/fs_context.c
>+++ b/fs/smb/client/fs_context.c
>@@ -1896,14 +1896,17 @@ void smb3_update_mnt_flags(struct cifs_sb_info *cifs_sb)
> 	if (ctx->mfsymlinks) {
> 		if (ctx->sfu_emul) {
> 			/*
>-			 * Our SFU ("Services for Unix" emulation does not allow
>-			 * creating symlinks but does allow reading existing SFU
>-			 * symlinks (it does allow both creating and reading SFU
>-			 * style mknod and FIFOs though). When "mfsymlinks" and
>+			 * Our SFU ("Services for Unix") emulation allows now
>+			 * creating new and reading existing SFU symlinks.
>+			 * Older Linux kernel versions were not able to neither
>+			 * read existing nor create new SFU symlinks. But
>+			 * creating and reading SFU style mknod and FIFOs was
>+			 * supported for long time. When "mfsymlinks" and
> 			 * "sfu" are both enabled at the same time, it allows
> 			 * reading both types of symlinks, but will only create
> 			 * them with mfsymlinks format. This allows better
>-			 * Apple compatibility (probably better for Samba too)
>+			 * Apple compatibility, compatibility with older Linux
>+			 * kernel clients (probably better for Samba too)
> 			 * while still recognizing old Windows style symlinks.
> 			 */
> 			cifs_dbg(VFS, "mount options mfsymlinks and sfu both enabled\n");
>diff --git a/fs/smb/client/link.c b/fs/smb/client/link.c
>index 80099bbb333b..47ddeb7fa111 100644
>--- a/fs/smb/client/link.c
>+++ b/fs/smb/client/link.c
>@@ -606,6 +606,9 @@ cifs_symlink(struct mnt_idmap *idmap, struct inode *inode,
> 	/* BB what if DFS and this volume is on different share? BB */
> 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) {
> 		rc = create_mf_symlink(xid, pTcon, cifs_sb, full_path, symname);
>+	} else if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) {
>+		rc = __cifs_sfu_make_node(xid, inode, direntry, pTcon,
>+					  full_path, S_IFLNK, 0, symname);
> #ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
> 	} else if (pTcon->unix_ext) {
> 		rc = CIFSUnixCreateSymLink(xid, pTcon, full_path, symname,
>diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
>index 9c2d065d3cc4..2c251e9a3a30 100644
>--- a/fs/smb/client/smb2ops.c
>+++ b/fs/smb/client/smb2ops.c
>@@ -5055,9 +5055,10 @@ static int smb2_next_header(struct TCP_Server_Info *server, char *buf,
> 	return 0;
> }
>
>-static 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)
>+				const char *full_path, umode_t mode, dev_t dev,
>+				const char *symname)
> {
> 	struct TCP_Server_Info *server = tcon->ses->server;
> 	struct cifs_open_parms oparms;
>@@ -5065,30 +5066,64 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
> 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
> 	struct cifs_fid fid;
> 	unsigned int bytes_written;
>-	struct win_dev pdev = {};
>-	struct kvec iov[2];
>+	u8 type[8];
>+	int type_len = 0;
>+	struct {
>+		__le64 major;
>+		__le64 minor;
>+	} __packed pdev = {};
>+	__le16 *symname_utf16 = NULL;
>+	u8 *data = NULL;
>+	int data_len = 0;
>+	struct kvec iov[3];
> 	__u32 oplock = server->oplocks ? REQ_OPLOCK : 0;
> 	int rc;
>
> 	switch (mode & S_IFMT) {
> 	case S_IFCHR:
>-		memcpy(pdev.type, "IntxCHR\0", 8);
>+		type_len = 8;
>+		memcpy(type, "IntxCHR\0", type_len);
> 		pdev.major = cpu_to_le64(MAJOR(dev));
> 		pdev.minor = cpu_to_le64(MINOR(dev));
>+		data = (u8 *)&pdev;
>+		data_len = sizeof(pdev);
> 		break;
> 	case S_IFBLK:
>-		memcpy(pdev.type, "IntxBLK\0", 8);
>+		type_len = 8;
>+		memcpy(type, "IntxBLK\0", type_len);
> 		pdev.major = cpu_to_le64(MAJOR(dev));
> 		pdev.minor = cpu_to_le64(MINOR(dev));
>+		data = (u8 *)&pdev;
>+		data_len = sizeof(pdev);
>+		break;
>+	case S_IFLNK:
>+		type_len = 8;
>+		memcpy(type, "IntxLNK\1", type_len);
>+		symname_utf16 = cifs_strndup_to_utf16(symname, strlen(symname),
>+						      &data_len, cifs_sb->local_nls,
>+						      NO_MAP_UNI_RSVD);
>+		if (!symname_utf16) {
>+			rc = -ENOMEM;
>+			goto out;
>+		}
>+		data_len -= 2; /* symlink is without trailing wide-nul */
>+		data = (u8 *)symname_utf16;

Can't S_IFLNK be handled somewhere else/other function?  mknod doesn't
support S_IFLNK, so this seems out of place.

And even though it's unreachable (AFAICS), cifs_sfu_make_node() calls
this with @symname == NULL (caught with gcc -fanalyzer).


Cheers,

Enzo

> 		break;
> 	case S_IFSOCK:
>-		strscpy(pdev.type, "LnxSOCK");
>+		type_len = 8;
>+		strscpy(type, "LnxSOCK");
>+		data = (u8 *)&pdev;
>+		data_len = sizeof(pdev);
> 		break;
> 	case S_IFIFO:
>-		strscpy(pdev.type, "LnxFIFO");
>+		type_len = 8;
>+		strscpy(type, "LnxFIFO");
>+		data = (u8 *)&pdev;
>+		data_len = sizeof(pdev);
> 		break;
> 	default:
>-		return -EPERM;
>+		rc = -EPERM;
>+		goto out;
> 	}
>
> 	oparms = CIFS_OPARMS(cifs_sb, tcon, full_path, GENERIC_WRITE,
>@@ -5098,17 +5133,26 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
>
> 	rc = server->ops->open(xid, &oparms, &oplock, NULL);
> 	if (rc)
>-		return rc;
>+		goto out;
>
>-	io_parms.pid = current->tgid;
>-	io_parms.tcon = tcon;
>-	io_parms.length = sizeof(pdev);
>-	iov[1].iov_base = &pdev;
>-	iov[1].iov_len = sizeof(pdev);
>+	if (type_len + data_len > 0) {
>+		io_parms.pid = current->tgid;
>+		io_parms.tcon = tcon;
>+		io_parms.length = type_len + data_len;
>+		iov[1].iov_base = type;
>+		iov[1].iov_len = type_len;
>+		iov[2].iov_base = data;
>+		iov[2].iov_len = data_len;
>+
>+		rc = server->ops->sync_write(xid, &fid, &io_parms,
>+					     &bytes_written,
>+					     iov, ARRAY_SIZE(iov)-1);
>+	}
>
>-	rc = server->ops->sync_write(xid, &fid, &io_parms,
>-				     &bytes_written, iov, 1);
> 	server->ops->close(xid, tcon, &fid);
>+
>+out:
>+	kfree(symname_utf16);
> 	return rc;
> }
>
>@@ -5120,7 +5164,7 @@ int cifs_sfu_make_node(unsigned int xid, struct inode *inode,
> 	int rc;
>
> 	rc = __cifs_sfu_make_node(xid, inode, dentry, tcon,
>-				  full_path, mode, dev);
>+				  full_path, mode, dev, NULL);
> 	if (rc)
> 		return rc;

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/4] cifs: Add support for creating SFU symlinks
  2024-09-27 17:54       ` Enzo Matsumiya
@ 2024-09-27 18:11         ` Pali Rohár
  0 siblings, 0 replies; 30+ messages in thread
From: Pali Rohár @ 2024-09-27 18:11 UTC (permalink / raw)
  To: Enzo Matsumiya
  Cc: Steve French, Paulo Alcantara, Ronnie Sahlberg, linux-cifs,
	linux-kernel

On Friday 27 September 2024 14:54:31 Enzo Matsumiya wrote:
> Hi Pali,
> 
> On 09/15, Pali Rohár wrote:
> > Linux cifs client can already detect SFU symlinks and reads it content
> > (target location). But currently is not able to create new symlink. So
> > implement this missing support.
> > 
> > When 'sfu' mount option is specified and 'mfsymlinks' is not specified then
> > create new symlinks in SFU-style. This will provide full SFU compatibility
> > of symlinks when mounting cifs share with 'sfu' option. 'mfsymlinks' option
> > override SFU for better Apple compatibility as explained in fs_context.c
> > file in smb3_update_mnt_flags() function.
> > 
> > Extend __cifs_sfu_make_node() function, which now can handle also S_IFLNK
> > type and refactor structures passed to sync_write() in this function, by
> > splitting SFU type and SFU data from original combined struct win_dev as
> > combined fixed-length struct cannot be used for variable-length symlinks.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> > fs/smb/client/cifspdu.h    |  6 ---
> > fs/smb/client/cifsproto.h  |  4 ++
> > fs/smb/client/fs_context.c | 13 ++++---
> > fs/smb/client/link.c       |  3 ++
> > fs/smb/client/smb2ops.c    | 80 +++++++++++++++++++++++++++++---------
> > 5 files changed, 77 insertions(+), 29 deletions(-)
> > 
> > diff --git a/fs/smb/client/cifspdu.h b/fs/smb/client/cifspdu.h
> > index a2072ab9e586..c3b6263060b0 100644
> > --- a/fs/smb/client/cifspdu.h
> > +++ b/fs/smb/client/cifspdu.h
> > @@ -2573,12 +2573,6 @@ typedef struct {
> > } __attribute__((packed)) FIND_FILE_STANDARD_INFO; /* level 0x1 FF resp data */
> > 
> > 
> > -struct win_dev {
> > -	unsigned char type[8]; /* IntxCHR or IntxBLK or LnxFIFO or LnxSOCK */
> > -	__le64 major;
> > -	__le64 minor;
> > -} __attribute__((packed));
> > -
> > struct fea {
> > 	unsigned char EA_flags;
> > 	__u8 name_len;
> > diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
> > index 497bf3c447bc..791bddac0396 100644
> > --- a/fs/smb/client/cifsproto.h
> > +++ b/fs/smb/client/cifsproto.h
> > @@ -676,6 +676,10 @@ char *extract_sharename(const char *unc);
> > int parse_reparse_point(struct reparse_data_buffer *buf,
> > 			u32 plen, struct cifs_sb_info *cifs_sb,
> > 			bool unicode, struct cifs_open_info_data *data);
> > +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,
> > +			 const char *symname);
> > 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);
> > diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
> > index bc926ab2555b..2f0c3894b0f7 100644
> > --- a/fs/smb/client/fs_context.c
> > +++ b/fs/smb/client/fs_context.c
> > @@ -1896,14 +1896,17 @@ void smb3_update_mnt_flags(struct cifs_sb_info *cifs_sb)
> > 	if (ctx->mfsymlinks) {
> > 		if (ctx->sfu_emul) {
> > 			/*
> > -			 * Our SFU ("Services for Unix" emulation does not allow
> > -			 * creating symlinks but does allow reading existing SFU
> > -			 * symlinks (it does allow both creating and reading SFU
> > -			 * style mknod and FIFOs though). When "mfsymlinks" and
> > +			 * Our SFU ("Services for Unix") emulation allows now
> > +			 * creating new and reading existing SFU symlinks.
> > +			 * Older Linux kernel versions were not able to neither
> > +			 * read existing nor create new SFU symlinks. But
> > +			 * creating and reading SFU style mknod and FIFOs was
> > +			 * supported for long time. When "mfsymlinks" and
> > 			 * "sfu" are both enabled at the same time, it allows
> > 			 * reading both types of symlinks, but will only create
> > 			 * them with mfsymlinks format. This allows better
> > -			 * Apple compatibility (probably better for Samba too)
> > +			 * Apple compatibility, compatibility with older Linux
> > +			 * kernel clients (probably better for Samba too)
> > 			 * while still recognizing old Windows style symlinks.
> > 			 */
> > 			cifs_dbg(VFS, "mount options mfsymlinks and sfu both enabled\n");
> > diff --git a/fs/smb/client/link.c b/fs/smb/client/link.c
> > index 80099bbb333b..47ddeb7fa111 100644
> > --- a/fs/smb/client/link.c
> > +++ b/fs/smb/client/link.c
> > @@ -606,6 +606,9 @@ cifs_symlink(struct mnt_idmap *idmap, struct inode *inode,
> > 	/* BB what if DFS and this volume is on different share? BB */
> > 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) {
> > 		rc = create_mf_symlink(xid, pTcon, cifs_sb, full_path, symname);
> > +	} else if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) {
> > +		rc = __cifs_sfu_make_node(xid, inode, direntry, pTcon,
> > +					  full_path, S_IFLNK, 0, symname);
> > #ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
> > 	} else if (pTcon->unix_ext) {
> > 		rc = CIFSUnixCreateSymLink(xid, pTcon, full_path, symname,
> > diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> > index 9c2d065d3cc4..2c251e9a3a30 100644
> > --- a/fs/smb/client/smb2ops.c
> > +++ b/fs/smb/client/smb2ops.c
> > @@ -5055,9 +5055,10 @@ static int smb2_next_header(struct TCP_Server_Info *server, char *buf,
> > 	return 0;
> > }
> > 
> > -static 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)
> > +				const char *full_path, umode_t mode, dev_t dev,
> > +				const char *symname)
> > {
> > 	struct TCP_Server_Info *server = tcon->ses->server;
> > 	struct cifs_open_parms oparms;
> > @@ -5065,30 +5066,64 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
> > 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
> > 	struct cifs_fid fid;
> > 	unsigned int bytes_written;
> > -	struct win_dev pdev = {};
> > -	struct kvec iov[2];
> > +	u8 type[8];
> > +	int type_len = 0;
> > +	struct {
> > +		__le64 major;
> > +		__le64 minor;
> > +	} __packed pdev = {};
> > +	__le16 *symname_utf16 = NULL;
> > +	u8 *data = NULL;
> > +	int data_len = 0;
> > +	struct kvec iov[3];
> > 	__u32 oplock = server->oplocks ? REQ_OPLOCK : 0;
> > 	int rc;
> > 
> > 	switch (mode & S_IFMT) {
> > 	case S_IFCHR:
> > -		memcpy(pdev.type, "IntxCHR\0", 8);
> > +		type_len = 8;
> > +		memcpy(type, "IntxCHR\0", type_len);
> > 		pdev.major = cpu_to_le64(MAJOR(dev));
> > 		pdev.minor = cpu_to_le64(MINOR(dev));
> > +		data = (u8 *)&pdev;
> > +		data_len = sizeof(pdev);
> > 		break;
> > 	case S_IFBLK:
> > -		memcpy(pdev.type, "IntxBLK\0", 8);
> > +		type_len = 8;
> > +		memcpy(type, "IntxBLK\0", type_len);
> > 		pdev.major = cpu_to_le64(MAJOR(dev));
> > 		pdev.minor = cpu_to_le64(MINOR(dev));
> > +		data = (u8 *)&pdev;
> > +		data_len = sizeof(pdev);
> > +		break;
> > +	case S_IFLNK:
> > +		type_len = 8;
> > +		memcpy(type, "IntxLNK\1", type_len);
> > +		symname_utf16 = cifs_strndup_to_utf16(symname, strlen(symname),
> > +						      &data_len, cifs_sb->local_nls,
> > +						      NO_MAP_UNI_RSVD);
> > +		if (!symname_utf16) {
> > +			rc = -ENOMEM;
> > +			goto out;
> > +		}
> > +		data_len -= 2; /* symlink is without trailing wide-nul */
> > +		data = (u8 *)symname_utf16;
> 
> Can't S_IFLNK be handled somewhere else/other function?  mknod doesn't
> support S_IFLNK, so this seems out of place.
> 
> And even though it's unreachable (AFAICS), cifs_sfu_make_node() calls
> this with @symname == NULL (caught with gcc -fanalyzer).
> 
> 
> Cheers,
> 
> Enzo

Hello! As SFU-style special files have same format, I just extended this
function which handles all SFU types, to handle also symlink.

I wanted to reuse existing function instead of copying and duplicating
code.

Parameter symname is used only for S_IFLNK, so this should be safe. And
as you pointed out mknod does not support S_IFLNK, so mknod code path
would not call __cifs_sfu_make_node() function with S_IFLNK argument.

So I think that there is not issue. But if you want to refactor this
code, do you have an idea what to do?

> > 		break;
> > 	case S_IFSOCK:
> > -		strscpy(pdev.type, "LnxSOCK");
> > +		type_len = 8;
> > +		strscpy(type, "LnxSOCK");
> > +		data = (u8 *)&pdev;
> > +		data_len = sizeof(pdev);
> > 		break;
> > 	case S_IFIFO:
> > -		strscpy(pdev.type, "LnxFIFO");
> > +		type_len = 8;
> > +		strscpy(type, "LnxFIFO");
> > +		data = (u8 *)&pdev;
> > +		data_len = sizeof(pdev);
> > 		break;
> > 	default:
> > -		return -EPERM;
> > +		rc = -EPERM;
> > +		goto out;
> > 	}
> > 
> > 	oparms = CIFS_OPARMS(cifs_sb, tcon, full_path, GENERIC_WRITE,
> > @@ -5098,17 +5133,26 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
> > 
> > 	rc = server->ops->open(xid, &oparms, &oplock, NULL);
> > 	if (rc)
> > -		return rc;
> > +		goto out;
> > 
> > -	io_parms.pid = current->tgid;
> > -	io_parms.tcon = tcon;
> > -	io_parms.length = sizeof(pdev);
> > -	iov[1].iov_base = &pdev;
> > -	iov[1].iov_len = sizeof(pdev);
> > +	if (type_len + data_len > 0) {
> > +		io_parms.pid = current->tgid;
> > +		io_parms.tcon = tcon;
> > +		io_parms.length = type_len + data_len;
> > +		iov[1].iov_base = type;
> > +		iov[1].iov_len = type_len;
> > +		iov[2].iov_base = data;
> > +		iov[2].iov_len = data_len;
> > +
> > +		rc = server->ops->sync_write(xid, &fid, &io_parms,
> > +					     &bytes_written,
> > +					     iov, ARRAY_SIZE(iov)-1);
> > +	}
> > 
> > -	rc = server->ops->sync_write(xid, &fid, &io_parms,
> > -				     &bytes_written, iov, 1);
> > 	server->ops->close(xid, tcon, &fid);
> > +
> > +out:
> > +	kfree(symname_utf16);
> > 	return rc;
> > }
> > 
> > @@ -5120,7 +5164,7 @@ int cifs_sfu_make_node(unsigned int xid, struct inode *inode,
> > 	int rc;
> > 
> > 	rc = __cifs_sfu_make_node(xid, inode, dentry, tcon,
> > -				  full_path, mode, dev);
> > +				  full_path, mode, dev, NULL);
> > 	if (rc)
> > 		return rc;

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2024-09-27 18:11 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-12 12:05 [PATCH 0/7] cifs: Improve client SFU support for special files Pali Rohár
2024-09-12 12:05 ` [PATCH 1/7] cifs: Fix recognizing SFU symlinks Pali Rohár
2024-09-13 20:04   ` Pali Rohár
2024-09-12 12:05 ` [PATCH 2/7] cifs: Add support for reading SFU symlink location Pali Rohár
2024-09-12 12:05 ` [PATCH 3/7] cifs: Put explicit zero byte into SFU block/char types Pali Rohár
2024-09-12 12:05 ` [PATCH 4/7] cifs: Show debug message when SFU Fifo type was detected Pali Rohár
2024-09-12 12:05 ` [PATCH 5/7] cifs: Recognize SFU socket type Pali Rohár
2024-09-12 12:05 ` [PATCH 6/7] cifs: Fix creating of SFU fifo and socket special files Pali Rohár
2024-09-13 20:07   ` Pali Rohár
2024-09-13 22:14     ` Steve French
2024-09-13 22:33       ` Steve French
2024-09-13 22:45         ` Pali Rohár
2024-09-13 22:42       ` Pali Rohár
2024-09-14  6:21         ` Steve French
2024-09-14  8:17           ` Pali Rohár
2024-09-15 17:41             ` Pali Rohár
     [not found]               ` <CAH2r5muXcyMxc=F2WsTtwQyKZ9TL64TWEBzX7bXJqZky2g0TzA@mail.gmail.com>
2024-09-15 17:48                 ` Pali Rohár
2024-09-16 16:23                   ` Pali Rohár
2024-09-15 19:45   ` [PATCH 0/4] cifs: Improve client SFU support for special files (2) Pali Rohár
2024-09-15 19:45     ` [PATCH 1/4] cifs: Add support for creating SFU symlinks Pali Rohár
2024-09-15 21:15       ` Steve French
2024-09-27 17:54       ` Enzo Matsumiya
2024-09-27 18:11         ` Pali Rohár
2024-09-15 19:45     ` [PATCH 2/4] cifs: Fix creating of SFU socket special files Pali Rohár
2024-09-15 19:45     ` [PATCH 3/4] cifs: Fix creating of SFU fifo " Pali Rohár
2024-09-15 19:45     ` [PATCH 4/4] cifs: Update SFU comments about fifos and sockets Pali Rohár
2024-09-15 21:14       ` Steve French
2024-09-12 12:05 ` [PATCH 7/7] cifs: Add support for creating SFU symlinks Pali Rohár
2024-09-13  3:44 ` [PATCH 0/7] cifs: Improve client SFU support for special files Steve French
2024-09-13  7:42   ` Pali Rohár

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox