Enzo,
I have lightly updated your patches. See attached.
1) smb: client: negotiate compression algorithms
Fixed a minor endian issue (with "arg" being le16 and being used to
compare vs. an int) and a few minor warnings spotted by checkpatch
(seq_puts is preferred instead of seq_printf for warning messages
without arguments)
2) smb: common: fix fields sizes in compression_pattern_payload_v1
added a brief description
3) smb: common: simplify compression headers
fixed a typo that checkpatch script caught (misaligned statement which
had extra space preceding it)
--
Thanks,
Steve
From 3b7fd184e2d09c21e62a82dccd7aff3c41de1f75 Mon Sep 17 00:00:00 2001
From: Enzo Matsumiya <ematsumiya@xxxxxxx>
Date: Fri, 8 Mar 2024 18:34:10 -0300
Subject: [PATCH 2/4] smb: common: fix fields sizes in
compression_pattern_payload_v1
See protocol documentation in MS-SMB2 section 2.2.42.2.2
Signed-off-by: Enzo Matsumiya <ematsumiya@xxxxxxx>
Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
---
fs/smb/common/smb2pdu.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/smb/common/smb2pdu.h b/fs/smb/common/smb2pdu.h
index 57f2343164a3..17f1bddb95d0 100644
--- a/fs/smb/common/smb2pdu.h
+++ b/fs/smb/common/smb2pdu.h
@@ -238,8 +238,8 @@ struct smb2_compression_transform_hdr_chained {
/* See MS-SMB2 2.2.42.2.2 */
struct compression_pattern_payload_v1 {
- __le16 Pattern;
- __le16 Reserved1;
+ __u8 Pattern;
+ __u8 Reserved1;
__le16 Reserved2;
__le32 Repetitions;
} __packed;
--
2.40.1
From 52ed13e3f7beaff8719994afded1bc5debebda23 Mon Sep 17 00:00:00 2001
From: Enzo Matsumiya <ematsumiya@xxxxxxx>
Date: Fri, 8 Mar 2024 19:00:12 -0300
Subject: [PATCH 3/4] smb: common: simplify compression headers
Unify compression headers (chained and unchained) into a single struct
so we can use it for the initial compression transform header
interchangeably.
Also make the OriginalPayloadSize field to be always visible in the
compression payload header, and have callers subtract its size when not
needed.
Rename the related structs to match the naming convetion used in the
other SMB2 structs.
Signed-off-by: Enzo Matsumiya <ematsumiya@xxxxxxx>
Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
---
fs/smb/common/smb2pdu.h | 45 ++++++++++++++++++++++++-----------------
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/fs/smb/common/smb2pdu.h b/fs/smb/common/smb2pdu.h
index 17f1bddb95d0..20784f76a604 100644
--- a/fs/smb/common/smb2pdu.h
+++ b/fs/smb/common/smb2pdu.h
@@ -208,36 +208,43 @@ struct smb2_transform_hdr {
__le64 SessionId;
} __packed;
+/*
+ * These are simplified versions from the spec, as we don't need a fully fledged
+ * form of both unchained and chained structs.
+ *
+ * Moreover, even in chained compressed payloads, the initial compression header
+ * has the form of the unchained one -- i.e. it never has the
+ * OriginalPayloadSize field and ::Offset field always represent an offset
+ * (instead of a length, as it is in the chained header).
+ *
+ * See MS-SMB2 2.2.42 for more details.
+ */
+#define SMB2_COMPRESSION_FLAG_NONE 0x0000
+#define SMB2_COMPRESSION_FLAG_CHAINED 0x0001
-/* See MS-SMB2 2.2.42 */
-struct smb2_compression_transform_hdr_unchained {
- __le32 ProtocolId; /* 0xFC 'S' 'M' 'B' */
+struct smb2_compression_hdr {
+ __le32 ProtocolId; /* 0xFC 'S' 'M' 'B' */
__le32 OriginalCompressedSegmentSize;
__le16 CompressionAlgorithm;
__le16 Flags;
- __le16 Length; /* if chained it is length, else offset */
+ __le16 Offset; /* this is the size of the uncompressed SMB2 header below */
+ /* uncompressed SMB2 header (READ or WRITE) goes here */
+ /* compressed data goes here */
} __packed;
-/* See MS-SMB2 2.2.42.1 */
-#define SMB2_COMPRESSION_FLAG_NONE 0x0000
-#define SMB2_COMPRESSION_FLAG_CHAINED 0x0001
-
-struct compression_payload_header {
+/*
+ * ... OTOH, set compression payload header to always have OriginalPayloadSize
+ * as it's easier to pass the struct size minus sizeof(OriginalPayloadSize)
+ * than to juggle around the header/data memory.
+ */
+struct smb2_compression_payload_hdr {
__le16 CompressionAlgorithm;
__le16 Flags;
__le32 Length; /* length of compressed playload including field below if present */
- /* __le32 OriginalPayloadSize; */ /* optional, present when LZNT1, LZ77, LZ77+Huffman */
-} __packed;
-
-/* See MS-SMB2 2.2.42.2 */
-struct smb2_compression_transform_hdr_chained {
- __le32 ProtocolId; /* 0xFC 'S' 'M' 'B' */
- __le32 OriginalCompressedSegmentSize;
- /* struct compression_payload_header[] */
+ __le32 OriginalPayloadSize; /* accounted when LZNT1, LZ77, LZ77+Huffman */
} __packed;
-/* See MS-SMB2 2.2.42.2.2 */
-struct compression_pattern_payload_v1 {
+struct smb2_compression_pattern_v1 {
__u8 Pattern;
__u8 Reserved1;
__le16 Reserved2;
--
2.40.1
From 8fea3473a1785a9fc2e6795f5785dd2ae3693bee Mon Sep 17 00:00:00 2001
From: Enzo Matsumiya <ematsumiya@xxxxxxx>
Date: Fri, 23 Feb 2024 11:58:57 -0300
Subject: [PATCH 1/4] smb: client: negotiate compression algorithms
Change "compress=" mount option to a boolean flag, that, if set,
will enable negotiating compression algorithms with the server.
Do not de/compress anything for now.
Signed-off-by: Enzo Matsumiya <ematsumiya@xxxxxxx>
Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
---
fs/smb/client/cifs_debug.c | 32 ++++++++++++++++++++++++++------
fs/smb/client/cifsglob.h | 6 +++++-
fs/smb/client/connect.c | 2 +-
fs/smb/client/fs_context.c | 2 +-
fs/smb/client/fs_context.h | 2 +-
fs/smb/client/smb2pdu.c | 20 +++++++++++++++-----
6 files changed, 49 insertions(+), 15 deletions(-)
diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
index 23d2622b969f..226d4835c92d 100644
--- a/fs/smb/client/cifs_debug.c
+++ b/fs/smb/client/cifs_debug.c
@@ -278,6 +278,24 @@ static int cifs_debug_files_proc_show(struct seq_file *m, void *v)
return 0;
}
+static __always_inline const char *compression_alg_str(__le16 alg)
+{
+ switch (alg) {
+ case SMB3_COMPRESS_NONE:
+ return "NONE";
+ case SMB3_COMPRESS_LZNT1:
+ return "LZNT1";
+ case SMB3_COMPRESS_LZ77:
+ return "LZ77";
+ case SMB3_COMPRESS_LZ77_HUFF:
+ return "LZ77-Huffman";
+ case SMB3_COMPRESS_PATTERN:
+ return "Pattern_V1";
+ default:
+ return "invalid";
+ }
+}
+
static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
{
struct mid_q_entry *mid_entry;
@@ -423,12 +441,6 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
server->echo_credits,
server->oplock_credits,
server->dialect);
- if (server->compress_algorithm == SMB3_COMPRESS_LZNT1)
- seq_printf(m, " COMPRESS_LZNT1");
- else if (server->compress_algorithm == SMB3_COMPRESS_LZ77)
- seq_printf(m, " COMPRESS_LZ77");
- else if (server->compress_algorithm == SMB3_COMPRESS_LZ77_HUFF)
- seq_printf(m, " COMPRESS_LZ77_HUFF");
if (server->sign)
seq_printf(m, " signed");
if (server->posix_ext_supported)
@@ -460,6 +472,14 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
server->leaf_fullpath);
}
+ seq_puts(m, "\nCompression: ");
+ if (!server->compression.requested)
+ seq_puts(m, "disabled on mount");
+ else if (server->compression.enabled)
+ seq_printf(m, "enabled (%s)", compression_alg_str(server->compression.alg));
+ else
+ seq_puts(m, "disabled (not supported by this server)");
+
seq_printf(m, "\n\n\tSessions: ");
i = 0;
list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index a0506a45eae3..8be62ed053a2 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -769,7 +769,11 @@ struct TCP_Server_Info {
unsigned int max_write;
unsigned int min_offload;
unsigned int retrans;
- __le16 compress_algorithm;
+ struct {
+ bool requested; /* "compress" mount option set*/
+ bool enabled; /* actually negotiated with server */
+ __le16 alg; /* preferred alg negotiated with server */
+ } compression;
__u16 signing_algorithm;
__le16 cipher_type;
/* save initital negprot hash */
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index 5d82921d63d1..86ae578904a2 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -1736,7 +1736,7 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx,
tcp_ses->channel_sequence_num = 0; /* only tracked for primary channel */
tcp_ses->reconnect_instance = 1;
tcp_ses->lstrp = jiffies;
- tcp_ses->compress_algorithm = cpu_to_le16(ctx->compression);
+ tcp_ses->compression.requested = ctx->compress;
spin_lock_init(&tcp_ses->req_lock);
spin_lock_init(&tcp_ses->srv_lock);
spin_lock_init(&tcp_ses->mid_lock);
diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index 4b767efa47f1..bdcbe6ff2739 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -963,7 +963,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
switch (opt) {
case Opt_compress:
- ctx->compression = UNKNOWN_TYPE;
+ ctx->compress = true;
cifs_dbg(VFS,
"SMB3 compression support is experimental\n");
break;
diff --git a/fs/smb/client/fs_context.h b/fs/smb/client/fs_context.h
index 1f09754977e7..7863f2248c4d 100644
--- a/fs/smb/client/fs_context.h
+++ b/fs/smb/client/fs_context.h
@@ -273,7 +273,7 @@ struct smb3_fs_context {
unsigned int max_credits; /* smb3 max_credits 10 < credits < 60000 */
unsigned int max_channels;
unsigned int max_cached_dirs;
- __u16 compression; /* compression algorithm 0xFFFF default 0=disabled */
+ bool compress; /* enable SMB2 messages (READ/WRITE) de/compression */
bool rootfs:1; /* if it's a SMB root file system */
bool witness:1; /* use witness protocol */
char *leaf_fullpath;
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 7d0157e0061e..60b3f8e41aa2 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -731,7 +731,7 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
pneg_ctxt += sizeof(struct smb2_posix_neg_context);
neg_context_count++;
- if (server->compress_algorithm) {
+ if (server->compression.requested) {
build_compression_ctxt((struct smb2_compression_capabilities_context *)
pneg_ctxt);
ctxt_len = ALIGN(sizeof(struct smb2_compression_capabilities_context), 8);
@@ -779,6 +779,9 @@ static void decode_compress_ctx(struct TCP_Server_Info *server,
struct smb2_compression_capabilities_context *ctxt)
{
unsigned int len = le16_to_cpu(ctxt->DataLength);
+ __le16 alg;
+
+ server->compression.enabled = false;
/*
* Caller checked that DataLength remains within SMB boundary. We still
@@ -789,15 +792,22 @@ static void decode_compress_ctx(struct TCP_Server_Info *server,
pr_warn_once("server sent bad compression cntxt\n");
return;
}
+
if (le16_to_cpu(ctxt->CompressionAlgorithmCount) != 1) {
- pr_warn_once("Invalid SMB3 compress algorithm count\n");
+ pr_warn_once("invalid SMB3 compress algorithm count\n");
return;
}
- if (le16_to_cpu(ctxt->CompressionAlgorithms[0]) > 3) {
- pr_warn_once("unknown compression algorithm\n");
+
+ alg = ctxt->CompressionAlgorithms[0];
+
+ /* 'NONE' (0) compressor type is never negotiated */
+ if (alg == 0 || le16_to_cpu(alg) > 3) {
+ pr_warn_once("invalid compression algorithm '%u'\n", alg);
return;
}
- server->compress_algorithm = ctxt->CompressionAlgorithms[0];
+
+ server->compression.alg = alg;
+ server->compression.enabled = true;
}
static int decode_encrypt_ctx(struct TCP_Server_Info *server,
--
2.40.1
