From: Dan Carpenter <dan.carpenter@linaro.org>
To: Jeffrey Hugo <quic_jhugo@quicinc.com>
Cc: Carl Vanderlip <quic_carlv@quicinc.com>,
Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>,
Oded Gabbay <ogabbay@kernel.org>,
Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>,
Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>,
linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
kernel-janitors@vger.kernel.org
Subject: [PATCH 2/5 v4] accel/qaic: tighten bounds checking in decode_message()
Date: Tue, 11 Jul 2023 11:20:54 +0300 [thread overview]
Message-ID: <ZK0Q5nbLyDO7kJa+@moroto> (raw)
In-Reply-To: <6e935c70-5bd2-4808-bdd9-d664f892b0b5@moroto.mountain>
Copy the bounds checking from encode_message() to decode_message().
This patch addresses the following concerns. Ensure that there is
enough space for at least one header so that we don't have a negative
size later.
if (msg_hdr_len < sizeof(*trans_hdr))
Ensure that we have enough space to read the next header from the
msg->data.
if (msg_len > msg_hdr_len - sizeof(*trans_hdr))
return -EINVAL;
Check that the trans_hdr->len is not below the minimum size:
if (hdr_len < sizeof(*trans_hdr))
This minimum check ensures that we don't corrupt memory in
decode_passthrough() when we do.
memcpy(out_trans->data, in_trans->data, len - sizeof(in_trans->hdr));
And finally, use size_add() to prevent an integer overflow:
if (size_add(msg_len, hdr_len) > msg_hdr_len)
Fixes: 129776ac2e38 ("accel/qaic: Add control path")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
v2: Fix the >= vs > bug in "if (msg_len > msg_hdr_len - sizeof(*trans_hdr))"
---
drivers/accel/qaic/qaic_control.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/accel/qaic/qaic_control.c b/drivers/accel/qaic/qaic_control.c
index 2fdd5959c52f..752b67aff777 100644
--- a/drivers/accel/qaic/qaic_control.c
+++ b/drivers/accel/qaic/qaic_control.c
@@ -956,15 +956,23 @@ static int decode_message(struct qaic_device *qdev, struct manage_msg *user_msg,
int ret;
int i;
- if (msg_hdr_len > QAIC_MANAGE_MAX_MSG_LENGTH)
+ if (msg_hdr_len < sizeof(*trans_hdr) ||
+ msg_hdr_len > QAIC_MANAGE_MAX_MSG_LENGTH)
return -EINVAL;
user_msg->len = 0;
user_msg->count = le32_to_cpu(msg->hdr.count);
for (i = 0; i < user_msg->count; ++i) {
+ u32 hdr_len;
+
+ if (msg_len > msg_hdr_len - sizeof(*trans_hdr))
+ return -EINVAL;
+
trans_hdr = (struct wire_trans_hdr *)(msg->data + msg_len);
- if (msg_len + le32_to_cpu(trans_hdr->len) > msg_hdr_len)
+ hdr_len = le32_to_cpu(trans_hdr->len);
+ if (hdr_len < sizeof(*trans_hdr) ||
+ size_add(msg_len, hdr_len) > msg_hdr_len)
return -EINVAL;
switch (le32_to_cpu(trans_hdr->type)) {
--
2.39.2
next prev parent reply other threads:[~2023-07-11 8:21 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-11 8:20 [PATCH 0/5 v4] accel/qaic: Improve bounds checking in encode/decode Dan Carpenter
2023-07-11 8:20 ` [PATCH 1/5 v4] accel/qaic: tighten bounds checking in encode_message() Dan Carpenter
2023-07-14 11:41 ` Pranjal Ramajor Asha Kanojiya
2023-07-14 16:02 ` Jeffrey Hugo
2023-07-11 8:20 ` Dan Carpenter [this message]
2023-07-14 11:42 ` [PATCH 2/5 v4] accel/qaic: tighten bounds checking in decode_message() Pranjal Ramajor Asha Kanojiya
2023-07-14 16:05 ` Jeffrey Hugo
2023-07-11 8:21 ` [PATCH 3/5 v4] accel/qaic: Add consistent integer overflow checks Dan Carpenter
2023-07-14 11:44 ` Pranjal Ramajor Asha Kanojiya
2023-07-14 16:14 ` Jeffrey Hugo
2023-07-11 8:21 ` [PATCH 4/5 v4] accel/qaic: move and expand integer overflow checks for map_user_pages() Dan Carpenter
2023-07-14 11:46 ` Pranjal Ramajor Asha Kanojiya
2023-07-11 8:21 ` [PATCH 5/5 v4] accel/qaic: Fix a leak in map_user_pages() Dan Carpenter
2023-07-14 11:47 ` Pranjal Ramajor Asha Kanojiya
2023-07-14 16:17 ` Jeffrey Hugo
2023-07-11 17:33 ` [PATCH 0/5 v4] accel/qaic: Improve bounds checking in encode/decode Jeffrey Hugo
2023-07-12 6:30 ` Dan Carpenter
2023-07-12 14:22 ` Jeffrey Hugo
2023-08-04 14:36 ` Jeffrey Hugo
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=ZK0Q5nbLyDO7kJa+@moroto \
--to=dan.carpenter@linaro.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=jacek.lawrynowicz@linux.intel.com \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=ogabbay@kernel.org \
--cc=quic_carlv@quicinc.com \
--cc=quic_jhugo@quicinc.com \
--cc=quic_pkanojiy@quicinc.com \
--cc=stanislaw.gruszka@linux.intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox