mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] accel/qaic: use sizeof(*trans_hdr) for transaction length check
@ 2026-06-17 21:25 Muhammad Bilal
  2026-07-29 20:52 ` Jeff Hugo
  2026-07-29 20:58 ` Jeff Hugo
  0 siblings, 2 replies; 3+ messages in thread
From: Muhammad Bilal @ 2026-06-17 21:25 UTC (permalink / raw)
  To: jeff.hugo
  Cc: carl.vanderlip, ogabbay, dan.carpenter, linux-arm-msm, dri-devel,
	linux-kernel, Muhammad Bilal

In encode_message() the per-transaction lower-bound check compares
trans_hdr->len against sizeof(trans_hdr), i.e. the size of the pointer,
instead of sizeof(*trans_hdr), the size of struct qaic_manage_trans_hdr.

Every other length check in this file (encode_message() at the loop
guard, decode_message(), etc.) correctly uses sizeof(*trans_hdr), so
this is an inconsistency. On 64-bit builds the pointer and the struct
are both 8 bytes, so the check is correct by coincidence and there is
no behavioural change. On 32-bit builds the pointer is 4 bytes, which
weakens the minimum-length check below the 8-byte header size.

Use sizeof(*trans_hdr) so the check validates against the actual
transaction header size on all builds.

Fixes: ea33cb6fc278 ("accel/qaic: tighten bounds checking in encode_message()")
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/accel/qaic/qaic_control.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/accel/qaic/qaic_control.c b/drivers/accel/qaic/qaic_control.c
index 43f84d4389602..4d4e789d5fcb8 100644
--- a/drivers/accel/qaic/qaic_control.c
+++ b/drivers/accel/qaic/qaic_control.c
@@ -786,7 +786,7 @@ static int encode_message(struct qaic_device *qdev, struct manage_msg *user_msg,
 			break;
 		}
 		trans_hdr = (struct qaic_manage_trans_hdr *)(user_msg->data + user_len);
-		if (trans_hdr->len < sizeof(trans_hdr) ||
+		if (trans_hdr->len < sizeof(*trans_hdr) ||
 		    size_add(user_len, trans_hdr->len) > user_msg->len) {
 			ret = -EINVAL;
 			break;
-- 
2.54.0


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

* Re: [PATCH] accel/qaic: use sizeof(*trans_hdr) for transaction length check
  2026-06-17 21:25 [PATCH] accel/qaic: use sizeof(*trans_hdr) for transaction length check Muhammad Bilal
@ 2026-07-29 20:52 ` Jeff Hugo
  2026-07-29 20:58 ` Jeff Hugo
  1 sibling, 0 replies; 3+ messages in thread
From: Jeff Hugo @ 2026-07-29 20:52 UTC (permalink / raw)
  To: Muhammad Bilal
  Cc: carl.vanderlip, ogabbay, dan.carpenter, linux-arm-msm, dri-devel,
	linux-kernel

On 6/17/2026 3:25 PM, Muhammad Bilal wrote:
> In encode_message() the per-transaction lower-bound check compares
> trans_hdr->len against sizeof(trans_hdr), i.e. the size of the pointer,
> instead of sizeof(*trans_hdr), the size of struct qaic_manage_trans_hdr.
> 
> Every other length check in this file (encode_message() at the loop
> guard, decode_message(), etc.) correctly uses sizeof(*trans_hdr), so
> this is an inconsistency. On 64-bit builds the pointer and the struct
> are both 8 bytes, so the check is correct by coincidence and there is
> no behavioural change. On 32-bit builds the pointer is 4 bytes, which
> weakens the minimum-length check below the 8-byte header size.
> 
> Use sizeof(*trans_hdr) so the check validates against the actual
> transaction header size on all builds.
> 
> Fixes: ea33cb6fc278 ("accel/qaic: tighten bounds checking in encode_message()")
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>

Reviewed-by: Jeff Hugo <jeff.hugo@oss.qualcomm.com>

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

* Re: [PATCH] accel/qaic: use sizeof(*trans_hdr) for transaction length check
  2026-06-17 21:25 [PATCH] accel/qaic: use sizeof(*trans_hdr) for transaction length check Muhammad Bilal
  2026-07-29 20:52 ` Jeff Hugo
@ 2026-07-29 20:58 ` Jeff Hugo
  1 sibling, 0 replies; 3+ messages in thread
From: Jeff Hugo @ 2026-07-29 20:58 UTC (permalink / raw)
  To: Muhammad Bilal
  Cc: carl.vanderlip, ogabbay, dan.carpenter, linux-arm-msm, dri-devel,
	linux-kernel

On 6/17/2026 3:25 PM, Muhammad Bilal wrote:
> In encode_message() the per-transaction lower-bound check compares
> trans_hdr->len against sizeof(trans_hdr), i.e. the size of the pointer,
> instead of sizeof(*trans_hdr), the size of struct qaic_manage_trans_hdr.
> 
> Every other length check in this file (encode_message() at the loop
> guard, decode_message(), etc.) correctly uses sizeof(*trans_hdr), so
> this is an inconsistency. On 64-bit builds the pointer and the struct
> are both 8 bytes, so the check is correct by coincidence and there is
> no behavioural change. On 32-bit builds the pointer is 4 bytes, which
> weakens the minimum-length check below the 8-byte header size.
> 
> Use sizeof(*trans_hdr) so the check validates against the actual
> transaction header size on all builds.
> 
> Fixes: ea33cb6fc278 ("accel/qaic: tighten bounds checking in encode_message()")
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>

Applied to drm-misc-fixes

-Jeff

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

end of thread, other threads:[~2026-07-29 21:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-17 21:25 [PATCH] accel/qaic: use sizeof(*trans_hdr) for transaction length check Muhammad Bilal
2026-07-29 20:52 ` Jeff Hugo
2026-07-29 20:58 ` Jeff Hugo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®