All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] smb: client: fix "df: Resource temporarily unavailable" on 5.10 stable kernel
@ 2024-01-26 19:31 kovalev
  2024-01-26 19:31 ` [PATCH 1/2] stddef: Introduce DECLARE_FLEX_ARRAY() helper kovalev
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: kovalev @ 2024-01-26 19:31 UTC (permalink / raw)
  To: stable, linux-doc, linux-kernel, linux-cifs, samba-technical
  Cc: keescook, sfrench, corbet, natechancellor, ndesaulniers, kovalev

After mounting a remote cifs resource, it becomes unavailable:
df: /mnt/sambashare: Resource temporarily unavailable

It was tested on the following Linux kernels:
Linux altlinux 5.10.208-std-def-alt1
Linux fedora 5.10.208-200.el8.x86_64

The error appeared starting from kernel 5.10.206 after adding
the commit [1] "smb: client: fix OOB in SMB2_query_info_init()",
in which the buffer length increases by 1 as a result of changes:
...
-      iov[0].iov_len = total_len - 1 + input_len;
+      iov[0].iov_len = len;
...

[1] https://patchwork.kernel.org/project/cifs-client/patch/20231213152557.6634-2-pc@manguebit.com/

Error fixed by backported commits in next two patches  adapted for the 5.10 kernel:

[PATCH 1/2] stddef: Introduce DECLARE_FLEX_ARRAY() helper
[PATCH 2/2] smb3: Replace smb2pdu 1-element arrays with flex-arrays


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

* [PATCH 1/2] stddef: Introduce DECLARE_FLEX_ARRAY() helper
  2024-01-26 19:31 [PATCH 0/2] smb: client: fix "df: Resource temporarily unavailable" on 5.10 stable kernel kovalev
@ 2024-01-26 19:31 ` kovalev
  2024-01-26 19:31 ` [PATCH 2/2] smb3: Replace smb2pdu 1-element arrays with flex-arrays kovalev
  2024-01-27  0:49 ` [PATCH 0/2] smb: client: fix "df: Resource temporarily unavailable" on 5.10 stable kernel Greg KH
  2 siblings, 0 replies; 9+ messages in thread
From: kovalev @ 2024-01-26 19:31 UTC (permalink / raw)
  To: stable, linux-doc, linux-kernel, linux-cifs, samba-technical
  Cc: keescook, sfrench, corbet, natechancellor, ndesaulniers, kovalev,
	Arnd Bergmann, Gustavo A. R. Silva

From: Kees Cook <keescook@chromium.org>

commit 3080ea5553cc909b000d1f1d964a9041962f2c5b upstream.

There are many places where kernel code wants to have several different
typed trailing flexible arrays. This would normally be done with multiple
flexible arrays in a union, but since GCC and Clang don't (on the surface)
allow this, there have been many open-coded workarounds, usually involving
neighboring 0-element arrays at the end of a structure. For example,
instead of something like this:

struct thing {
	...
	union {
		struct type1 foo[];
		struct type2 bar[];
	};
};

code works around the compiler with:

struct thing {
	...
	struct type1 foo[0];
	struct type2 bar[];
};

Another case is when a flexible array is wanted as the single member
within a struct (which itself is usually in a union). For example, this
would be worked around as:

union many {
	...
	struct {
		struct type3 baz[0];
	};
};

These kinds of work-arounds cause problems with size checks against such
zero-element arrays (for example when building with -Warray-bounds and
-Wzero-length-bounds, and with the coming FORTIFY_SOURCE improvements),
so they must all be converted to "real" flexible arrays, avoiding warnings
like this:

fs/hpfs/anode.c: In function 'hpfs_add_sector_to_btree':
fs/hpfs/anode.c:209:27: warning: array subscript 0 is outside the bounds of an interior zero-length array 'struct bplus_internal_node[0]' [-Wzero-length-bounds]
  209 |    anode->btree.u.internal[0].down = cpu_to_le32(a);
      |    ~~~~~~~~~~~~~~~~~~~~~~~^~~
In file included from fs/hpfs/hpfs_fn.h:26,
                 from fs/hpfs/anode.c:10:
fs/hpfs/hpfs.h:412:32: note: while referencing 'internal'
  412 |     struct bplus_internal_node internal[0]; /* (internal) 2-word entries giving
      |                                ^~~~~~~~

drivers/net/can/usb/etas_es58x/es58x_fd.c: In function 'es58x_fd_tx_can_msg':
drivers/net/can/usb/etas_es58x/es58x_fd.c:360:35: warning: array subscript 65535 is outside the bounds of an interior zero-length array 'u8[0]' {aka 'unsigned char[]'} [-Wzero-length-bounds]
  360 |  tx_can_msg = (typeof(tx_can_msg))&es58x_fd_urb_cmd->raw_msg[msg_len];
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/net/can/usb/etas_es58x/es58x_core.h:22,
                 from drivers/net/can/usb/etas_es58x/es58x_fd.c:17:
drivers/net/can/usb/etas_es58x/es58x_fd.h:231:6: note: while referencing 'raw_msg'
  231 |   u8 raw_msg[0];
      |      ^~~~~~~

However, it _is_ entirely possible to have one or more flexible arrays
in a struct or union: it just has to be in another struct. And since it
cannot be alone in a struct, such a struct must have at least 1 other
named member -- but that member can be zero sized. Wrap all this nonsense
into the new DECLARE_FLEX_ARRAY() in support of having flexible arrays
in unions (or alone in a struct).

As with struct_group(), since this is needed in UAPI headers as well,
implement the core there, with a non-UAPI wrapper.

Additionally update kernel-doc to understand its existence.

https://github.com/KSPP/linux/issues/137

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.org>
---
 include/linux/stddef.h      | 13 +++++++++++++
 include/uapi/linux/stddef.h | 16 ++++++++++++++++
 scripts/kernel-doc          |  3 ++-
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index 938216f8ab7e7c..31fdbb784c24e2 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -84,4 +84,17 @@ enum {
 #define struct_group_tagged(TAG, NAME, MEMBERS...) \
 	__struct_group(TAG, NAME, /* no attrs */, MEMBERS)
 
+/**
+ * DECLARE_FLEX_ARRAY() - Declare a flexible array usable in a union
+ *
+ * @TYPE: The type of each flexible array element
+ * @NAME: The name of the flexible array member
+ *
+ * In order to have a flexible array member in a union or alone in a
+ * struct, it needs to be wrapped in an anonymous struct with at least 1
+ * named member, but that member can be empty.
+ */
+#define DECLARE_FLEX_ARRAY(TYPE, NAME) \
+	__DECLARE_FLEX_ARRAY(TYPE, NAME)
+
 #endif
diff --git a/include/uapi/linux/stddef.h b/include/uapi/linux/stddef.h
index c3725b49226323..7837ba4fe72890 100644
--- a/include/uapi/linux/stddef.h
+++ b/include/uapi/linux/stddef.h
@@ -28,4 +28,20 @@
 		struct { MEMBERS } ATTRS; \
 		struct TAG { MEMBERS } ATTRS NAME; \
 	}
+
+/**
+ * __DECLARE_FLEX_ARRAY() - Declare a flexible array usable in a union
+ *
+ * @TYPE: The type of each flexible array element
+ * @NAME: The name of the flexible array member
+ *
+ * In order to have a flexible array member in a union or alone in a
+ * struct, it needs to be wrapped in an anonymous struct with at least 1
+ * named member, but that member can be empty.
+ */
+#define __DECLARE_FLEX_ARRAY(TYPE, NAME)	\
+	struct { \
+		struct { } __empty_ ## NAME; \
+		TYPE NAME[]; \
+	}
 #endif
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 19af6dd160e6b7..7a04d4c0532607 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1232,7 +1232,8 @@ sub dump_struct($$) {
 	$members =~ s/DECLARE_KFIFO\s*\(([^,)]+),\s*([^,)]+),\s*([^,)]+)\)/$2 \*$1/gos;
 	# replace DECLARE_KFIFO_PTR
 	$members =~ s/DECLARE_KFIFO_PTR\s*\(([^,)]+),\s*([^,)]+)\)/$2 \*$1/gos;
-
+	# replace DECLARE_FLEX_ARRAY
+	$members =~ s/(?:__)?DECLARE_FLEX_ARRAY\s*\($args,\s*$args\)/$1 $2\[\]/gos;
 	my $declaration = $members;
 
 	# Split nested struct/union elements as newer ones
-- 
2.33.8


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

* [PATCH 2/2] smb3: Replace smb2pdu 1-element arrays with flex-arrays
  2024-01-26 19:31 [PATCH 0/2] smb: client: fix "df: Resource temporarily unavailable" on 5.10 stable kernel kovalev
  2024-01-26 19:31 ` [PATCH 1/2] stddef: Introduce DECLARE_FLEX_ARRAY() helper kovalev
@ 2024-01-26 19:31 ` kovalev
  2024-01-27  0:49 ` [PATCH 0/2] smb: client: fix "df: Resource temporarily unavailable" on 5.10 stable kernel Greg KH
  2 siblings, 0 replies; 9+ messages in thread
From: kovalev @ 2024-01-26 19:31 UTC (permalink / raw)
  To: stable, linux-doc, linux-kernel, linux-cifs, samba-technical
  Cc: keescook, sfrench, corbet, natechancellor, ndesaulniers, kovalev,
	Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	Namjae Jeon, Sergey Senozhatsky, Steve French

From: Kees Cook <keescook@chromium.org>

commit eb3e28c1e89b4984308777231887e41aa8a0151f upstream.

The kernel is globally removing the ambiguous 0-length and 1-element
arrays in favor of flexible arrays, so that we can gain both compile-time
and run-time array bounds checking[1].

Replace the trailing 1-element array with a flexible array in the
following structures:

        struct smb2_err_rsp
        struct smb2_tree_connect_req
        struct smb2_negotiate_rsp
        struct smb2_sess_setup_req
        struct smb2_sess_setup_rsp
        struct smb2_read_req
        struct smb2_read_rsp
        struct smb2_write_req
        struct smb2_write_rsp
        struct smb2_query_directory_req
        struct smb2_query_directory_rsp
        struct smb2_set_info_req
        struct smb2_change_notify_rsp
        struct smb2_create_rsp
        struct smb2_query_info_req
        struct smb2_query_info_rsp

Replace the trailing 1-element array with a flexible array, but leave
the existing structure padding:

        struct smb2_file_all_info
        struct smb2_lock_req

Adjust all related size calculations to match the changes to sizeof().

No machine code output or .data section differences are produced after
these changes.

[1] For lots of details, see both:
    https://docs.kernel.org/process/deprecated.html#zero-length-and-one-element-arrays
    https://people.kernel.org/kees/bounded-flexible-arrays-in-c

Cc: Steve French <sfrench@samba.org>
Cc: Paulo Alcantara <pc@cjr.nz>
Cc: Ronnie Sahlberg <lsahlber@redhat.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.org>
---
 fs/cifs/smb2misc.c |  2 +-
 fs/cifs/smb2ops.c  | 14 +++++++-------
 fs/cifs/smb2pdu.c  | 13 ++++++-------
 fs/cifs/smb2pdu.h  | 42 ++++++++++++++++++++++++------------------
 4 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
index b98bba887f84b0..660e00eb42060a 100644
--- a/fs/cifs/smb2misc.c
+++ b/fs/cifs/smb2misc.c
@@ -117,7 +117,7 @@ static __u32 get_neg_ctxt_len(struct smb2_sync_hdr *hdr, __u32 len,
 	} else if (nc_offset + 1 == non_ctxlen) {
 		cifs_dbg(FYI, "no SPNEGO security blob in negprot rsp\n");
 		size_of_pad_before_neg_ctxts = 0;
-	} else if (non_ctxlen == SMB311_NEGPROT_BASE_SIZE)
+	} else if (non_ctxlen == SMB311_NEGPROT_BASE_SIZE + 1)
 		/* has padding, but no SPNEGO blob */
 		size_of_pad_before_neg_ctxts = nc_offset - non_ctxlen + 1;
 	else
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 26edaeb4245d8c..84850a55c8b7e7 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -5561,7 +5561,7 @@ struct smb_version_values smb20_values = {
 	.header_size = sizeof(struct smb2_sync_hdr),
 	.header_preamble_size = 0,
 	.max_header_size = MAX_SMB2_HDR_SIZE,
-	.read_rsp_size = sizeof(struct smb2_read_rsp) - 1,
+	.read_rsp_size = sizeof(struct smb2_read_rsp),
 	.lock_cmd = SMB2_LOCK,
 	.cap_unix = 0,
 	.cap_nt_find = SMB2_NT_FIND,
@@ -5583,7 +5583,7 @@ struct smb_version_values smb21_values = {
 	.header_size = sizeof(struct smb2_sync_hdr),
 	.header_preamble_size = 0,
 	.max_header_size = MAX_SMB2_HDR_SIZE,
-	.read_rsp_size = sizeof(struct smb2_read_rsp) - 1,
+	.read_rsp_size = sizeof(struct smb2_read_rsp),
 	.lock_cmd = SMB2_LOCK,
 	.cap_unix = 0,
 	.cap_nt_find = SMB2_NT_FIND,
@@ -5604,7 +5604,7 @@ struct smb_version_values smb3any_values = {
 	.header_size = sizeof(struct smb2_sync_hdr),
 	.header_preamble_size = 0,
 	.max_header_size = MAX_SMB2_HDR_SIZE,
-	.read_rsp_size = sizeof(struct smb2_read_rsp) - 1,
+	.read_rsp_size = sizeof(struct smb2_read_rsp),
 	.lock_cmd = SMB2_LOCK,
 	.cap_unix = 0,
 	.cap_nt_find = SMB2_NT_FIND,
@@ -5625,7 +5625,7 @@ struct smb_version_values smbdefault_values = {
 	.header_size = sizeof(struct smb2_sync_hdr),
 	.header_preamble_size = 0,
 	.max_header_size = MAX_SMB2_HDR_SIZE,
-	.read_rsp_size = sizeof(struct smb2_read_rsp) - 1,
+	.read_rsp_size = sizeof(struct smb2_read_rsp),
 	.lock_cmd = SMB2_LOCK,
 	.cap_unix = 0,
 	.cap_nt_find = SMB2_NT_FIND,
@@ -5646,7 +5646,7 @@ struct smb_version_values smb30_values = {
 	.header_size = sizeof(struct smb2_sync_hdr),
 	.header_preamble_size = 0,
 	.max_header_size = MAX_SMB2_HDR_SIZE,
-	.read_rsp_size = sizeof(struct smb2_read_rsp) - 1,
+	.read_rsp_size = sizeof(struct smb2_read_rsp),
 	.lock_cmd = SMB2_LOCK,
 	.cap_unix = 0,
 	.cap_nt_find = SMB2_NT_FIND,
@@ -5667,7 +5667,7 @@ struct smb_version_values smb302_values = {
 	.header_size = sizeof(struct smb2_sync_hdr),
 	.header_preamble_size = 0,
 	.max_header_size = MAX_SMB2_HDR_SIZE,
-	.read_rsp_size = sizeof(struct smb2_read_rsp) - 1,
+	.read_rsp_size = sizeof(struct smb2_read_rsp),
 	.lock_cmd = SMB2_LOCK,
 	.cap_unix = 0,
 	.cap_nt_find = SMB2_NT_FIND,
@@ -5688,7 +5688,7 @@ struct smb_version_values smb311_values = {
 	.header_size = sizeof(struct smb2_sync_hdr),
 	.header_preamble_size = 0,
 	.max_header_size = MAX_SMB2_HDR_SIZE,
-	.read_rsp_size = sizeof(struct smb2_read_rsp) - 1,
+	.read_rsp_size = sizeof(struct smb2_read_rsp),
 	.lock_cmd = SMB2_LOCK,
 	.cap_unix = 0,
 	.cap_nt_find = SMB2_NT_FIND,
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 76679dc4e63288..4aec01841f0f26 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1261,7 +1261,7 @@ SMB2_sess_sendreceive(struct SMB2_sess_data *sess_data)
 
 	/* Testing shows that buffer offset must be at location of Buffer[0] */
 	req->SecurityBufferOffset =
-		cpu_to_le16(sizeof(struct smb2_sess_setup_req) - 1 /* pad */);
+		cpu_to_le16(sizeof(struct smb2_sess_setup_req));
 	req->SecurityBufferLength = cpu_to_le16(sess_data->iov[1].iov_len);
 
 	memset(&rqst, 0, sizeof(struct smb_rqst));
@@ -1760,8 +1760,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
 	iov[0].iov_len = total_len - 1;
 
 	/* Testing shows that buffer offset must be at location of Buffer[0] */
-	req->PathOffset = cpu_to_le16(sizeof(struct smb2_tree_connect_req)
-			- 1 /* pad */);
+	req->PathOffset = cpu_to_le16(sizeof(struct smb2_tree_connect_req));
 	req->PathLength = cpu_to_le16(unc_path_len - 2);
 	iov[1].iov_base = unc_path;
 	iov[1].iov_len = unc_path_len;
@@ -4676,7 +4675,7 @@ int SMB2_query_directory_init(const unsigned int xid,
 	memcpy(bufptr, &asteriks, len);
 
 	req->FileNameOffset =
-		cpu_to_le16(sizeof(struct smb2_query_directory_req) - 1);
+		cpu_to_le16(sizeof(struct smb2_query_directory_req));
 	req->FileNameLength = cpu_to_le16(len);
 	/*
 	 * BB could be 30 bytes or so longer if we used SMB2 specific
@@ -4873,7 +4872,7 @@ SMB2_set_info_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
 	req->AdditionalInformation = cpu_to_le32(additional_info);
 
 	req->BufferOffset =
-			cpu_to_le16(sizeof(struct smb2_set_info_req) - 1);
+			cpu_to_le16(sizeof(struct smb2_set_info_req));
 	req->BufferLength = cpu_to_le32(*size);
 
 	memcpy(req->Buffer, *data, *size);
@@ -5105,9 +5104,9 @@ build_qfs_info_req(struct kvec *iov, struct cifs_tcon *tcon,
 	req->VolatileFileId = volatile_fid;
 	/* 1 for pad */
 	req->InputBufferOffset =
-			cpu_to_le16(sizeof(struct smb2_query_info_req) - 1);
+			cpu_to_le16(sizeof(struct smb2_query_info_req));
 	req->OutputBufferLength = cpu_to_le32(
-		outbuf_len + sizeof(struct smb2_query_info_rsp) - 1);
+		outbuf_len + sizeof(struct smb2_query_info_rsp));
 
 	iov->iov_base = (char *)req;
 	iov->iov_len = total_len;
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index 89a732b31390ea..eaa873175318a0 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -220,7 +220,7 @@ struct smb2_err_rsp {
 	__le16 StructureSize;
 	__le16 Reserved; /* MBZ */
 	__le32 ByteCount;  /* even if zero, at least one byte follows */
-	__u8   ErrorData[1];  /* variable length */
+	__u8   ErrorData[];  /* variable length */
 } __packed;
 
 #define SYMLINK_ERROR_TAG 0x4c4d5953
@@ -464,7 +464,7 @@ struct smb2_negotiate_rsp {
 	__le16 SecurityBufferOffset;
 	__le16 SecurityBufferLength;
 	__le32 NegotiateContextOffset;	/* Pre:SMB3.1.1 was reserved/ignored */
-	__u8   Buffer[1];	/* variable length GSS security buffer */
+	__u8   Buffer[];	/* variable length GSS security buffer */
 } __packed;
 
 /* Flags */
@@ -481,7 +481,7 @@ struct smb2_sess_setup_req {
 	__le16 SecurityBufferOffset;
 	__le16 SecurityBufferLength;
 	__u64 PreviousSessionId;
-	__u8   Buffer[1];	/* variable length GSS security buffer */
+	__u8   Buffer[];	/* variable length GSS security buffer */
 } __packed;
 
 /* Currently defined SessionFlags */
@@ -494,7 +494,7 @@ struct smb2_sess_setup_rsp {
 	__le16 SessionFlags;
 	__le16 SecurityBufferOffset;
 	__le16 SecurityBufferLength;
-	__u8   Buffer[1];	/* variable length GSS security buffer */
+	__u8   Buffer[];	/* variable length GSS security buffer */
 } __packed;
 
 struct smb2_logoff_req {
@@ -520,7 +520,7 @@ struct smb2_tree_connect_req {
 	__le16 Flags; /* Reserved MBZ for dialects prior to SMB3.1.1 */
 	__le16 PathOffset;
 	__le16 PathLength;
-	__u8   Buffer[1];	/* variable length */
+	__u8   Buffer[];	/* variable length */
 } __packed;
 
 /* See MS-SMB2 section 2.2.9.2 */
@@ -828,7 +828,7 @@ struct smb2_create_rsp {
 	__u64  VolatileFileId; /* opaque endianness */
 	__le32 CreateContextsOffset;
 	__le32 CreateContextsLength;
-	__u8   Buffer[1];
+	__u8   Buffer[];
 } __packed;
 
 struct create_context {
@@ -1289,7 +1289,7 @@ struct smb2_read_plain_req {
 	__le32 RemainingBytes;
 	__le16 ReadChannelInfoOffset;
 	__le16 ReadChannelInfoLength;
-	__u8   Buffer[1];
+	__u8   Buffer[];
 } __packed;
 
 /* Read flags */
@@ -1304,7 +1304,7 @@ struct smb2_read_rsp {
 	__le32 DataLength;
 	__le32 DataRemaining;
 	__u32  Flags;
-	__u8   Buffer[1];
+	__u8   Buffer[];
 } __packed;
 
 /* For write request Flags field below the following flags are defined: */
@@ -1324,7 +1324,7 @@ struct smb2_write_req {
 	__le16 WriteChannelInfoOffset;
 	__le16 WriteChannelInfoLength;
 	__le32 Flags;
-	__u8   Buffer[1];
+	__u8   Buffer[];
 } __packed;
 
 struct smb2_write_rsp {
@@ -1335,7 +1335,7 @@ struct smb2_write_rsp {
 	__le32 DataLength;
 	__le32 DataRemaining;
 	__u32  Reserved2;
-	__u8   Buffer[1];
+	__u8   Buffer[];
 } __packed;
 
 /* notify flags */
@@ -1371,7 +1371,7 @@ struct smb2_change_notify_rsp {
 	__le16	StructureSize;  /* Must be 9 */
 	__le16	OutputBufferOffset;
 	__le32	OutputBufferLength;
-	__u8	Buffer[1]; /* array of file notify structs */
+	__u8	Buffer[]; /* array of file notify structs */
 } __packed;
 
 #define SMB2_LOCKFLAG_SHARED_LOCK	0x0001
@@ -1394,7 +1394,10 @@ struct smb2_lock_req {
 	__u64  PersistentFileId; /* opaque endianness */
 	__u64  VolatileFileId; /* opaque endianness */
 	/* Followed by at least one */
-	struct smb2_lock_element locks[1];
+	union {
+		struct smb2_lock_element lock;
+		DECLARE_FLEX_ARRAY(struct smb2_lock_element, locks);
+	};
 } __packed;
 
 struct smb2_lock_rsp {
@@ -1434,7 +1437,7 @@ struct smb2_query_directory_req {
 	__le16 FileNameOffset;
 	__le16 FileNameLength;
 	__le32 OutputBufferLength;
-	__u8   Buffer[1];
+	__u8   Buffer[];
 } __packed;
 
 struct smb2_query_directory_rsp {
@@ -1442,7 +1445,7 @@ struct smb2_query_directory_rsp {
 	__le16 StructureSize; /* Must be 9 */
 	__le16 OutputBufferOffset;
 	__le32 OutputBufferLength;
-	__u8   Buffer[1];
+	__u8   Buffer[];
 } __packed;
 
 /* Possible InfoType values */
@@ -1483,7 +1486,7 @@ struct smb2_query_info_req {
 	__le32 Flags;
 	__u64  PersistentFileId; /* opaque endianness */
 	__u64  VolatileFileId; /* opaque endianness */
-	__u8   Buffer[1];
+	__u8   Buffer[];
 } __packed;
 
 struct smb2_query_info_rsp {
@@ -1491,7 +1494,7 @@ struct smb2_query_info_rsp {
 	__le16 StructureSize; /* Must be 9 */
 	__le16 OutputBufferOffset;
 	__le32 OutputBufferLength;
-	__u8   Buffer[1];
+	__u8   Buffer[];
 } __packed;
 
 /*
@@ -1514,7 +1517,7 @@ struct smb2_set_info_req {
 	__le32 AdditionalInformation;
 	__u64  PersistentFileId; /* opaque endianness */
 	__u64  VolatileFileId; /* opaque endianness */
-	__u8   Buffer[1];
+	__u8   Buffer[];
 } __packed;
 
 struct smb2_set_info_rsp {
@@ -1716,7 +1719,10 @@ struct smb2_file_all_info { /* data block encoding of response to level 18 */
 	__le32 Mode;
 	__le32 AlignmentRequirement;
 	__le32 FileNameLength;
-	char   FileName[1];
+	union {
+		char __pad;     /* Legacy structure padding */
+		DECLARE_FLEX_ARRAY(char, FileName);
+	};
 } __packed; /* level 18 Query */
 
 struct smb2_file_eof_info { /* encoding of request for level 10 */
-- 
2.33.8


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

* Re: [PATCH 0/2] smb: client: fix "df: Resource temporarily unavailable" on 5.10 stable kernel
  2024-01-26 19:31 [PATCH 0/2] smb: client: fix "df: Resource temporarily unavailable" on 5.10 stable kernel kovalev
  2024-01-26 19:31 ` [PATCH 1/2] stddef: Introduce DECLARE_FLEX_ARRAY() helper kovalev
  2024-01-26 19:31 ` [PATCH 2/2] smb3: Replace smb2pdu 1-element arrays with flex-arrays kovalev
@ 2024-01-27  0:49 ` Greg KH
  2024-01-27  6:42   ` Harshit Mogalapalli
  2 siblings, 1 reply; 9+ messages in thread
From: Greg KH @ 2024-01-27  0:49 UTC (permalink / raw)
  To: kovalev
  Cc: stable, linux-doc, linux-kernel, linux-cifs, samba-technical,
	keescook, sfrench, corbet, natechancellor, ndesaulniers

On Fri, Jan 26, 2024 at 10:31:41PM +0300, kovalev@altlinux.org wrote:
> After mounting a remote cifs resource, it becomes unavailable:
> df: /mnt/sambashare: Resource temporarily unavailable
> 
> It was tested on the following Linux kernels:
> Linux altlinux 5.10.208-std-def-alt1
> Linux fedora 5.10.208-200.el8.x86_64
> 
> The error appeared starting from kernel 5.10.206 after adding
> the commit [1] "smb: client: fix OOB in SMB2_query_info_init()",
> in which the buffer length increases by 1 as a result of changes:
> ...
> -      iov[0].iov_len = total_len - 1 + input_len;
> +      iov[0].iov_len = len;
> ...
> 
> [1] https://patchwork.kernel.org/project/cifs-client/patch/20231213152557.6634-2-pc@manguebit.com/
> 
> Error fixed by backported commits in next two patches  adapted for the 5.10 kernel:
> 
> [PATCH 1/2] stddef: Introduce DECLARE_FLEX_ARRAY() helper
> [PATCH 2/2] smb3: Replace smb2pdu 1-element arrays with flex-arrays
> 
> 

Now queued up, thanks.

greg k-h

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

* Re: [PATCH 0/2] smb: client: fix "df: Resource temporarily unavailable" on 5.10 stable kernel
  2024-01-27  0:49 ` [PATCH 0/2] smb: client: fix "df: Resource temporarily unavailable" on 5.10 stable kernel Greg KH
@ 2024-01-27  6:42   ` Harshit Mogalapalli
  2024-01-27  8:02     ` kovalev
  2024-01-27 23:01     ` Steve French
  0 siblings, 2 replies; 9+ messages in thread
From: Harshit Mogalapalli @ 2024-01-27  6:42 UTC (permalink / raw)
  To: kovalev, Greg KH
  Cc: stable, linux-doc, linux-kernel, linux-cifs, samba-technical,
	keescook, sfrench, corbet, natechancellor, ndesaulniers, pc,
	Mohamed Abuelfotoh, Hazem, Shyam Prasad N, Vegard Nossum,
	Darren Kenny

Hi,

Adding more people to CC.(who have looked at this issue)

On 27/01/24 6:19 am, Greg KH wrote:
> On Fri, Jan 26, 2024 at 10:31:41PM +0300, kovalev@altlinux.org wrote:
>> After mounting a remote cifs resource, it becomes unavailable:
>> df: /mnt/sambashare: Resource temporarily unavailable
>>
>> It was tested on the following Linux kernels:
>> Linux altlinux 5.10.208-std-def-alt1
>> Linux fedora 5.10.208-200.el8.x86_64
>>
>> The error appeared starting from kernel 5.10.206 after adding
>> the commit [1] "smb: client: fix OOB in SMB2_query_info_init()",
>> in which the buffer length increases by 1 as a result of changes:
>> ...
>> -      iov[0].iov_len = total_len - 1 + input_len;
>> +      iov[0].iov_len = len;
>> ...
>>

We can reproduce this on 5.15.148(latest 5.15.y) and Mohamed reported 
this on 6.1.y, so we need backports there as well.

https://lore.kernel.org/all/09738f0f-53a2-43f1-a09d-a2bef48e1344@oracle.com/


[root@vm1 xfstests-dev]# ./check -g quick -s smb3
TEST_DEV=//<SERVER_IP>/TEST is mounted but not a type cifs filesystem
[root@vm1 xfstests-dev]# df
df: /mnt/test: Resource temporarily unavailable


This two patch series doesn't cleanly apply to 5.15.y.

Also I am unsure, which is the better approach to go with

Approach 1 - suggested by Paulo:
https://lore.kernel.org/all/446860c571d0699ed664175262a9e84b@manguebit.com/

Approach 2 - this series
Pulling in [PATCH 2/2] smb3: Replace smb2pdu 1-element arrays with 
flex-arrays like this series did.

I think approach 1 is better as the changes are minimal, but please 
correct me if that seems wrong.

Thanks,
Harshit
>> [1] https://patchwork.kernel.org/project/cifs-client/patch/20231213152557.6634-2-pc@manguebit.com/
>>
>> Error fixed by backported commits in next two patches  adapted for the 5.10 kernel:
>>
>> [PATCH 1/2] stddef: Introduce DECLARE_FLEX_ARRAY() helper
>> [PATCH 2/2] smb3: Replace smb2pdu 1-element arrays with flex-arrays
>>
>>
> 
> Now queued up, thanks.
> 
> greg k-h
> 


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

* Re: [PATCH 0/2] smb: client: fix "df: Resource temporarily unavailable" on 5.10 stable kernel
  2024-01-27  6:42   ` Harshit Mogalapalli
@ 2024-01-27  8:02     ` kovalev
  2024-01-27 13:29       ` Harshit Mogalapalli
  2024-01-27 23:01     ` Steve French
  1 sibling, 1 reply; 9+ messages in thread
From: kovalev @ 2024-01-27  8:02 UTC (permalink / raw)
  To: Harshit Mogalapalli, Greg KH
  Cc: stable, linux-doc, linux-kernel, linux-cifs, samba-technical,
	keescook, sfrench, corbet, natechancellor, ndesaulniers, pc,
	Mohamed Abuelfotoh, Hazem, Shyam Prasad N, Vegard Nossum,
	Darren Kenny

Hi,

27.01.2024 09:42, Harshit Mogalapalli wrote:
> We can reproduce this on 5.15.148(latest 5.15.y) and Mohamed reported 
> this on 6.1.y, so we need backports there as well.

in the 6.1.72 kernel, this problem was fixed by the commit [1] "smb3: 
Replace smb2pdu 1-element arrays with flex-arrays", which was proposed 
in this series of patches.


[1] https://lore.kernel.org/all/2024010937-eggplant-bauble-d556@gregkh/T/

-- 
Regards,
Vasiliy Kovalev


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

* Re: [PATCH 0/2] smb: client: fix "df: Resource temporarily unavailable" on 5.10 stable kernel
  2024-01-27  8:02     ` kovalev
@ 2024-01-27 13:29       ` Harshit Mogalapalli
  2024-01-27 21:20         ` Greg KH
  0 siblings, 1 reply; 9+ messages in thread
From: Harshit Mogalapalli @ 2024-01-27 13:29 UTC (permalink / raw)
  To: kovalev, Greg KH
  Cc: stable, linux-doc, linux-kernel, linux-cifs, samba-technical,
	keescook, sfrench, corbet, natechancellor, ndesaulniers, pc,
	Mohamed Abuelfotoh, Hazem, Shyam Prasad N, Vegard Nossum,
	Darren Kenny, linkinjeon

Hi Kovalev,

On 27/01/24 1:32 pm, kovalev@altlinux.org wrote:
> Hi,
> 
> 27.01.2024 09:42, Harshit Mogalapalli wrote:
>> We can reproduce this on 5.15.148(latest 5.15.y) and Mohamed reported 
>> this on 6.1.y, so we need backports there as well.
> 
> in the 6.1.72 kernel, this problem was fixed by the commit [1] "smb3: 
> Replace smb2pdu 1-element arrays with flex-arrays", which was proposed 
> in this series of patches.
> 
Thanks for sharing this, I didnot notice that the above commit was 
backported to 6.1.72.

I think we still need fixing in 5.15.y as the commit eb3e28c1e89b 
("smb3: Replace smb2pdu 1-element arrays with flex-arrays") is not in 
5.15.148

Thanks,
Harshit
> 
> [1] https://lore.kernel.org/all/2024010937-eggplant-bauble-d556@gregkh/T/
> 


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

* Re: [PATCH 0/2] smb: client: fix "df: Resource temporarily unavailable" on 5.10 stable kernel
  2024-01-27 13:29       ` Harshit Mogalapalli
@ 2024-01-27 21:20         ` Greg KH
  0 siblings, 0 replies; 9+ messages in thread
From: Greg KH @ 2024-01-27 21:20 UTC (permalink / raw)
  To: Harshit Mogalapalli
  Cc: kovalev, stable, linux-doc, linux-kernel, linux-cifs,
	samba-technical, keescook, sfrench, corbet, natechancellor,
	ndesaulniers, pc, Mohamed Abuelfotoh, Hazem, Shyam Prasad N,
	Vegard Nossum, Darren Kenny, linkinjeon

On Sat, Jan 27, 2024 at 06:59:15PM +0530, Harshit Mogalapalli wrote:
> Hi Kovalev,
> 
> On 27/01/24 1:32 pm, kovalev@altlinux.org wrote:
> > Hi,
> > 
> > 27.01.2024 09:42, Harshit Mogalapalli wrote:
> > > We can reproduce this on 5.15.148(latest 5.15.y) and Mohamed
> > > reported this on 6.1.y, so we need backports there as well.
> > 
> > in the 6.1.72 kernel, this problem was fixed by the commit [1] "smb3:
> > Replace smb2pdu 1-element arrays with flex-arrays", which was proposed
> > in this series of patches.
> > 
> Thanks for sharing this, I didnot notice that the above commit was
> backported to 6.1.72.
> 
> I think we still need fixing in 5.15.y as the commit eb3e28c1e89b ("smb3:
> Replace smb2pdu 1-element arrays with flex-arrays") is not in 5.15.148

Patches gladly accepted :)

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

* Re: [PATCH 0/2] smb: client: fix "df: Resource temporarily unavailable" on 5.10 stable kernel
  2024-01-27  6:42   ` Harshit Mogalapalli
  2024-01-27  8:02     ` kovalev
@ 2024-01-27 23:01     ` Steve French
  1 sibling, 0 replies; 9+ messages in thread
From: Steve French @ 2024-01-27 23:01 UTC (permalink / raw)
  To: Harshit Mogalapalli
  Cc: kovalev, Greg KH, stable, linux-doc, linux-kernel, linux-cifs,
	samba-technical, keescook, sfrench, corbet, natechancellor,
	ndesaulniers, pc, Mohamed Abuelfotoh, Hazem, Shyam Prasad N,
	Vegard Nossum, Darren Kenny

On Sat, Jan 27, 2024 at 12:43 AM Harshit Mogalapalli
<harshit.m.mogalapalli@oracle.com> wrote:
>
> Hi,
>
> Adding more people to CC.(who have looked at this issue)
>
> On 27/01/24 6:19 am, Greg KH wrote:
> > On Fri, Jan 26, 2024 at 10:31:41PM +0300, kovalev@altlinux.org wrote:
> >> After mounting a remote cifs resource, it becomes unavailable:
> >> df: /mnt/sambashare: Resource temporarily unavailable
> >>
> >> It was tested on the following Linux kernels:
> >> Linux altlinux 5.10.208-std-def-alt1
> >> Linux fedora 5.10.208-200.el8.x86_64
> >>
> >> The error appeared starting from kernel 5.10.206 after adding
> >> the commit [1] "smb: client: fix OOB in SMB2_query_info_init()",
> >> in which the buffer length increases by 1 as a result of changes:
> >> ...
> >> -      iov[0].iov_len = total_len - 1 + input_len;
> >> +      iov[0].iov_len = len;
> >> ...
> >>
>
> We can reproduce this on 5.15.148(latest 5.15.y) and Mohamed reported
> this on 6.1.y, so we need backports there as well.
>
> https://lore.kernel.org/all/09738f0f-53a2-43f1-a09d-a2bef48e1344@oracle.com/
>
>
> [root@vm1 xfstests-dev]# ./check -g quick -s smb3
> TEST_DEV=//<SERVER_IP>/TEST is mounted but not a type cifs filesystem
> [root@vm1 xfstests-dev]# df
> df: /mnt/test: Resource temporarily unavailable
>
>
> This two patch series doesn't cleanly apply to 5.15.y.
>
> Also I am unsure, which is the better approach to go with
>
> Approach 1 - suggested by Paulo:
> https://lore.kernel.org/all/446860c571d0699ed664175262a9e84b@manguebit.com/
>
> Approach 2 - this series
> Pulling in [PATCH 2/2] smb3: Replace smb2pdu 1-element arrays with
> flex-arrays like this series did.
>
> I think approach 1 is better as the changes are minimal, but please
> correct me if that seems wrong.

Yes - Paulo's fix looks simple


-- 
Thanks,

Steve

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

end of thread, other threads:[~2024-01-27 23:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-26 19:31 [PATCH 0/2] smb: client: fix "df: Resource temporarily unavailable" on 5.10 stable kernel kovalev
2024-01-26 19:31 ` [PATCH 1/2] stddef: Introduce DECLARE_FLEX_ARRAY() helper kovalev
2024-01-26 19:31 ` [PATCH 2/2] smb3: Replace smb2pdu 1-element arrays with flex-arrays kovalev
2024-01-27  0:49 ` [PATCH 0/2] smb: client: fix "df: Resource temporarily unavailable" on 5.10 stable kernel Greg KH
2024-01-27  6:42   ` Harshit Mogalapalli
2024-01-27  8:02     ` kovalev
2024-01-27 13:29       ` Harshit Mogalapalli
2024-01-27 21:20         ` Greg KH
2024-01-27 23:01     ` Steve French

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.