Commit eb3e28c1 authored by Kees Cook's avatar Kees Cook Committed by Steve French
Browse files

smb3: Replace smb2pdu 1-element arrays with flex-arrays 

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: default avatarNamjae Jeon <linkinjeon@kernel.org>
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent 0e9bd27b

fs/cifs/smb2file.c

+1 −1
Original line number Diff line number Diff line
@@ -34,7 +34,7 @@ static struct smb2_symlink_err_rsp *symlink_data(const struct kvec *iov) 
		len = (u32)err->ErrorContextCount * (offsetof(struct smb2_error_context_rsp,

							      ErrorContextData) +

						     sizeof(struct smb2_symlink_err_rsp));

		if (le32_to_cpu(err->ByteCount) < len || iov->iov_len < len + sizeof(*err))

		if (le32_to_cpu(err->ByteCount) < len || iov->iov_len < len + sizeof(*err) + 1)

			return ERR_PTR(-EINVAL);



		p = (struct smb2_error_context_rsp *)err->ErrorData;

fs/cifs/smb2misc.c

+1 −1
Original line number Diff line number Diff line
@@ -113,7 +113,7 @@ static __u32 get_neg_ctxt_len(struct smb2_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

fs/cifs/smb2ops.c

+7 −7
Original line number Diff line number Diff line
@@ -5645,7 +5645,7 @@ struct smb_version_values smb20_values = { 
	.header_size = sizeof(struct smb2_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 smb21_values = { 
	.header_size = sizeof(struct smb2_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 smb3any_values = { 
	.header_size = sizeof(struct smb2_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,
@@ -5709,7 +5709,7 @@ struct smb_version_values smbdefault_values = { 
	.header_size = sizeof(struct smb2_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,
@@ -5730,7 +5730,7 @@ struct smb_version_values smb30_values = { 
	.header_size = sizeof(struct smb2_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,
@@ -5751,7 +5751,7 @@ struct smb_version_values smb302_values = { 
	.header_size = sizeof(struct smb2_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,
@@ -5772,7 +5772,7 @@ struct smb_version_values smb311_values = { 
	.header_size = sizeof(struct smb2_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,

fs/cifs/smb2pdu.c

+7 −9
Original line number Diff line number Diff line
@@ -1373,7 +1373,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));
@@ -1892,8 +1892,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);

	iov[1].iov_base = unc_path;

	iov[1].iov_len = unc_path_len;
@@ -3773,7 +3772,7 @@ SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon, 
			ses->Suid, (u8)watch_tree, completion_filter);

		/* validate that notify information is plausible */

		if ((rsp_iov.iov_base == NULL) ||

		    (rsp_iov.iov_len < sizeof(struct smb2_change_notify_rsp)))

		    (rsp_iov.iov_len < sizeof(struct smb2_change_notify_rsp) + 1))

			goto cnotify_exit;



		smb_rsp = (struct smb2_change_notify_rsp *)rsp_iov.iov_base;
@@ -4966,7 +4965,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
@@ -5162,8 +5161,7 @@ SMB2_set_info_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server, 
	req->VolatileFileId = volatile_fid;

	req->AdditionalInformation = cpu_to_le32(additional_info);



	req->BufferOffset =

			cpu_to_le16(sizeof(struct smb2_set_info_req) - 1);

	req->BufferOffset = cpu_to_le16(sizeof(struct smb2_set_info_req));

	req->BufferLength = cpu_to_le32(*size);



	memcpy(req->Buffer, *data, *size);
@@ -5397,9 +5395,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;

fs/cifs/smb2pdu.h

+1 −1
Original line number Diff line number Diff line
@@ -57,7 +57,7 @@ struct smb2_rdma_crypto_transform { 
#define COMPOUND_FID 0xFFFFFFFFFFFFFFFFULL



#define SMB2_SYMLINK_STRUCT_SIZE \

	(sizeof(struct smb2_err_rsp) - 1 + sizeof(struct smb2_symlink_err_rsp))

	(sizeof(struct smb2_err_rsp) + sizeof(struct smb2_symlink_err_rsp))



#define SYMLINK_ERROR_TAG 0x4c4d5953