All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thorsten Blum <thorsten.blum@linux.dev>
To: Namjae Jeon <linkinjeon@kernel.org>,
	Steve French <sfrench@samba.org>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Tom Talpey <tom@talpey.com>, Kees Cook <kees@kernel.org>,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Thorsten Blum <thorsten.blum@linux.dev>,
	linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-hardening@vger.kernel.org
Subject: [PATCH v2] ksmbd: Annotate struct copychunk_ioctl_req with __counted_by_le()
Date: Wed, 25 Sep 2024 11:03:13 +0200	[thread overview]
Message-ID: <20240925090313.22310-2-thorsten.blum@linux.dev> (raw)

Add the __counted_by_le compiler attribute to the flexible array member
Chunks to improve access bounds-checking via CONFIG_UBSAN_BOUNDS and
CONFIG_FORTIFY_SOURCE.

Change the data type of the flexible array member Chunks from __u8[] to
struct srv_copychunk[] for ChunkCount to match the number of elements in
the Chunks array. (With __u8[], each srv_copychunk would occupy 24 array
entries and the __counted_by compiler attribute wouldn't be applicable.)

Use struct_size() to calculate the size of the copychunk_ioctl_req.

Read Chunks[0] after checking that ChunkCount is not 0.

Compile-tested only.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
Changes in v2:
- Change the data type of Chunks from __u8[] to struct srv_copychunk[]
  as suggested by Tom Talpey
- Use struct_size()
- Link to v1: https://lore.kernel.org/linux-kernel/20240924102243.239811-2-thorsten.blum@linux.dev/
---
 fs/smb/server/smb2pdu.c |  5 ++---
 fs/smb/server/smb2pdu.h | 14 +++++++-------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 7121266daa02..62312c5e790e 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -7566,7 +7566,6 @@ static int fsctl_copychunk(struct ksmbd_work *work,
 	ci_rsp->TotalBytesWritten =
 		cpu_to_le32(ksmbd_server_side_copy_max_total_size());
 
-	chunks = (struct srv_copychunk *)&ci_req->Chunks[0];
 	chunk_count = le32_to_cpu(ci_req->ChunkCount);
 	if (chunk_count == 0)
 		goto out;
@@ -7574,12 +7573,12 @@ static int fsctl_copychunk(struct ksmbd_work *work,
 
 	/* verify the SRV_COPYCHUNK_COPY packet */
 	if (chunk_count > ksmbd_server_side_copy_max_chunk_count() ||
-	    input_count < offsetof(struct copychunk_ioctl_req, Chunks) +
-	     chunk_count * sizeof(struct srv_copychunk)) {
+	    input_count < struct_size(ci_req, Chunks, chunk_count)) {
 		rsp->hdr.Status = STATUS_INVALID_PARAMETER;
 		return -EINVAL;
 	}
 
+	chunks = &ci_req->Chunks[0];
 	for (i = 0; i < chunk_count; i++) {
 		if (le32_to_cpu(chunks[i].Length) == 0 ||
 		    le32_to_cpu(chunks[i].Length) > ksmbd_server_side_copy_max_chunk_size())
diff --git a/fs/smb/server/smb2pdu.h b/fs/smb/server/smb2pdu.h
index 73aff20e22d0..649dacf7e8c4 100644
--- a/fs/smb/server/smb2pdu.h
+++ b/fs/smb/server/smb2pdu.h
@@ -190,13 +190,6 @@ struct resume_key_ioctl_rsp {
 	__u8 Context[4]; /* ignored, Windows sets to 4 bytes of zero */
 } __packed;
 
-struct copychunk_ioctl_req {
-	__le64 ResumeKey[3];
-	__le32 ChunkCount;
-	__le32 Reserved;
-	__u8 Chunks[]; /* array of srv_copychunk */
-} __packed;
-
 struct srv_copychunk {
 	__le64 SourceOffset;
 	__le64 TargetOffset;
@@ -204,6 +197,13 @@ struct srv_copychunk {
 	__le32 Reserved;
 } __packed;
 
+struct copychunk_ioctl_req {
+	__le64 ResumeKey[3];
+	__le32 ChunkCount;
+	__le32 Reserved;
+	struct srv_copychunk Chunks[] __counted_by_le(ChunkCount);
+} __packed;
+
 struct copychunk_ioctl_rsp {
 	__le32 ChunksWritten;
 	__le32 ChunkBytesWritten;
-- 
2.46.1


             reply	other threads:[~2024-09-25  9:10 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-25  9:03 Thorsten Blum [this message]
2024-09-28  0:53 ` [PATCH v2] ksmbd: Annotate struct copychunk_ioctl_req with __counted_by_le() Namjae Jeon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240925090313.22310-2-thorsten.blum@linux.dev \
    --to=thorsten.blum@linux.dev \
    --cc=gustavoars@kernel.org \
    --cc=kees@kernel.org \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=senozhatsky@chromium.org \
    --cc=sfrench@samba.org \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.