[PATCH] RDMA/rxe: bound the WQE opcode before indexing rxe_wr_opcode_info[]

[PATCH] RDMA/rxe: bound the WQE opcode before indexing rxe_wr_opcode_info[]

[Youngsung Ahn]

wr_opcode_mask() indexes the global rxe_wr_opcode_info[] array with
a caller-supplied opcode and no bounds check. For a user QP the WQE
comes from an mmap'd SQ ring, so wqe->wr.opcode is an
attacker-controlled __u32 read back by the requester
(req_next_wqe() -> rxe_requester()); rxe_post_send() takes the
qp->is_user branch and never runs validate_send_wr(), whose only
opcode check is itself !wr_opcode_mask() (it indexes before it
checks). rxe_wr_opcode_info[] has entries only up to IB_WR_REG_MR,
so an out-of-range opcode reads out of bounds and the result is used
as a mask and dereferenced; observed as a KASAN global-out-of-bounds
"Read of size 4" and a wild-pointer oops.

Return a zero mask for an out-of-range opcode, matching how
validate_send_wr() already treats a zero mask (an invalid WR), so no
path indexes the array out of bounds.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Youngsung Ahn <ays511.kr@gmail.com>
---
Notes (not part of the commit):
Reproduced on a KASAN x86-64 build of 7.3-rc4 as uid 1000. Present unchanged
in mainline 551c722f4080 (2026-09-29) and rdma for-next. Compile-tested
(KASAN+RDMA_RXE), not runtime-tested. Found through manual review; per
security-bugs.rst this is public and a reproducer can be shared on request.
IB_WR_REG_MR is the highest index the array initializes; an
ARRAY_SIZE(rxe_wr_opcode_info) bound would be equivalent but the array is
extern to this header.

 drivers/infiniband/sw/rxe/rxe_loc.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index 64d636bf80fd..dcc0788db90a 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -184,6 +184,9 @@ void rxe_comp_queue_pkt(struct rxe_qp *qp, struct sk_buff *skb);

 static inline unsigned int wr_opcode_mask(int opcode, struct rxe_qp *qp)
 {
+	if (unlikely(opcode < 0 || opcode > IB_WR_REG_MR))
+		return 0;
+
 	return rxe_wr_opcode_info[opcode].mask[qp->ibqp.qp_type];
 }

--
2.43.0

Re: [PATCH] RDMA/rxe: bound the WQE opcode before indexing rxe_wr_opcode_info[]

[sashiko-bot]

> wr_opcode_mask() indexes the global rxe_wr_opcode_info[] array with
> a caller-supplied opcode and no bounds check. For a user QP the WQE
> comes from an mmap'd SQ ring, so wqe->wr.opcode is an
> attacker-controlled __u32 read back by the requester
> (req_next_wqe() -> rxe_requester()); rxe_post_send() takes the
> qp->is_user branch and never runs validate_send_wr(), whose only
> opcode check is itself !wr_opcode_mask() (it indexes before it
> checks). rxe_wr_opcode_info[] has entries only up to IB_WR_REG_MR,
> so an out-of-range opcode reads out of bounds and the result is used
> as a mask and dereferenced; observed as a KASAN global-out-of-bounds
> "Read of size 4" and a wild-pointer oops.
> 
> Return a zero mask for an out-of-range opcode, matching how
> validate_send_wr() already treats a zero mask (an invalid WR), so no
> path indexes the array out of bounds.
> 
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Youngsung Ahn <ays511.kr@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930180221.2497908-1-ays511.kr@gmail.com?part=1

[PATCH v2] RDMA/rxe: bound the WQE opcode before indexing rxe_wr_opcode_info[]

[Youngsung Ahn]

wr_opcode_mask() indexes the global rxe_wr_opcode_info[] array with
a caller-supplied opcode and no bounds check. For a user QP the WQE
comes from an mmap'd SQ ring, so wqe->wr.opcode is an
attacker-controlled __u32 read back by the requester
(req_next_wqe() -> rxe_requester()); rxe_post_send() takes the
qp->is_user branch and never runs validate_send_wr(), whose only
opcode check is itself !wr_opcode_mask() (it indexes before it
checks). rxe_wr_opcode_info[] has entries only up to IB_WR_REG_MR,
so an out-of-range opcode reads out of bounds and the result is used
as a mask and dereferenced; observed as a KASAN global-out-of-bounds
"Read of size 4" and a wild-pointer oops.

Return a zero mask for an out-of-range opcode, matching how
validate_send_wr() already treats a zero mask (an invalid WR), so no
path indexes the array out of bounds.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Cc: stable@vger.kernel.org
Signed-off-by: Youngsung Ahn <ays511.kr@gmail.com>
Assisted-by: LLM
---
Notes (not part of the commit):
Reproduced on a KASAN x86-64 build of 7.3-rc4 as uid 1000. Present unchanged
in mainline 551c722f4080 (2026-09-29) and rdma for-next. Compile-tested
(KASAN+RDMA_RXE), not runtime-tested. Per security-bugs.rst this is public and
a reproducer can be shared on request. IB_WR_REG_MR is the highest index the
array initializes; an ARRAY_SIZE(rxe_wr_opcode_info) bound would be equivalent
but the array is extern to this header.

v2: no code change. The irregular indentation in v1 was email mangling (HTML),
not a coding-style problem -- as Willy Tarreau noted on the ODP thread; resent
as plain text with tabs preserved via git send-email. Added Cc: stable, fixed
the author/Signed-off-by match, and added Assisted-by: LLM.

 drivers/infiniband/sw/rxe/rxe_loc.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index 64d636bf80fd..dcc0788db90a 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -184,6 +184,9 @@ void rxe_comp_queue_pkt(struct rxe_qp *qp, struct sk_buff *skb);

 static inline unsigned int wr_opcode_mask(int opcode, struct rxe_qp *qp)
 {
+	if (unlikely(opcode < 0 || opcode > IB_WR_REG_MR))
+		return 0;
+
 	return rxe_wr_opcode_info[opcode].mask[qp->ibqp.qp_type];
 }

--
2.43.0

Re: [PATCH v2] RDMA/rxe: bound the WQE opcode before indexing rxe_wr_opcode_info[]

[sashiko-bot]

> wr_opcode_mask() indexes the global rxe_wr_opcode_info[] array with
> a caller-supplied opcode and no bounds check. For a user QP the WQE
> comes from an mmap'd SQ ring, so wqe->wr.opcode is an
> attacker-controlled __u32 read back by the requester
> (req_next_wqe() -> rxe_requester()); rxe_post_send() takes the
> qp->is_user branch and never runs validate_send_wr(), whose only
> opcode check is itself !wr_opcode_mask() (it indexes before it
> checks). rxe_wr_opcode_info[] has entries only up to IB_WR_REG_MR,
> so an out-of-range opcode reads out of bounds and the result is used
> as a mask and dereferenced; observed as a KASAN global-out-of-bounds
> "Read of size 4" and a wild-pointer oops.
> 
> Return a zero mask for an out-of-range opcode, matching how
> validate_send_wr() already treats a zero mask (an invalid WR), so no
> path indexes the array out of bounds.
> 
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Youngsung Ahn <ays511.kr@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001152407.3259261-1-ays511.kr@gmail.com?part=1

Re: [PATCH v2] RDMA/rxe: bound the WQE opcode before indexing rxe_wr_opcode_info[]

[Zhu Yanjun]

在 2026/10/1 8:24, Youngsung Ahn 写道:
> wr_opcode_mask() indexes the global rxe_wr_opcode_info[] array with
> a caller-supplied opcode and no bounds check. For a user QP the WQE
> comes from an mmap'd SQ ring, so wqe->wr.opcode is an
> attacker-controlled __u32 read back by the requester
> (req_next_wqe() -> rxe_requester()); rxe_post_send() takes the
> qp->is_user branch and never runs validate_send_wr(), whose only
> opcode check is itself !wr_opcode_mask() (it indexes before it
> checks). rxe_wr_opcode_info[] has entries only up to IB_WR_REG_MR,
> so an out-of-range opcode reads out of bounds and the result is used
> as a mask and dereferenced; observed as a KASAN global-out-of-bounds
> "Read of size 4" and a wild-pointer oops.
> 
> Return a zero mask for an out-of-range opcode, matching how
> validate_send_wr() already treats a zero mask (an invalid WR), so no
> path indexes the array out of bounds.
> 
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Youngsung Ahn <ays511.kr@gmail.com>
> Assisted-by: LLM
> ---
> Notes (not part of the commit):
> Reproduced on a KASAN x86-64 build of 7.3-rc4 as uid 1000. Present unchanged
> in mainline 551c722f4080 (2026-09-29) and rdma for-next. Compile-tested
> (KASAN+RDMA_RXE), not runtime-tested. Per security-bugs.rst this is public and
> a reproducer can be shared on request. IB_WR_REG_MR is the highest index the
> array initializes; an ARRAY_SIZE(rxe_wr_opcode_info) bound would be equivalent
> but the array is extern to this header.

The function wr_opcode_mask() is used in the following three functions:
"
validate_send_wr()
req_retry()
req_next_wqe()
"

In validate_send_wr() and req_next_wqe(), if wr_opcode_mask() returns 0, 
it is relatively clear from the surrounding code how the invalid opcode 
is handled.

However, for req_retry(), it is less clear to me what the subsequent 
control flow is when wr_opcode_mask() returns 0. In particular, I would 
like to understand whether returning 0 for an out-of-range opcode is 
sufficient to safely handle the invalid WQE in this path.

You mentioned that you were able to reproduce the issue on your local 
host. Would you be able to share the reproducer with us? It would help 
us better understand the failure path and verify the fix, especially for 
the req_retry() case.

Also, please follow Leon's advice and send v2 as a standalone patch.

Thanks a lot.
Yanjun Zhu

> 
> v2: no code change. The irregular indentation in v1 was email mangling (HTML),
> not a coding-style problem -- as Willy Tarreau noted on the ODP thread; resent
> as plain text with tabs preserved via git send-email. Added Cc: stable, fixed
> the author/Signed-off-by match, and added Assisted-by: LLM.
> 
>   drivers/infiniband/sw/rxe/rxe_loc.h | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
> index 64d636bf80fd..dcc0788db90a 100644
> --- a/drivers/infiniband/sw/rxe/rxe_loc.h
> +++ b/drivers/infiniband/sw/rxe/rxe_loc.h
> @@ -184,6 +184,9 @@ void rxe_comp_queue_pkt(struct rxe_qp *qp, struct sk_buff *skb);
> 
>   static inline unsigned int wr_opcode_mask(int opcode, struct rxe_qp *qp)
>   {
> +	if (unlikely(opcode < 0 || opcode > IB_WR_REG_MR))
> +		return 0;
> +
>   	return rxe_wr_opcode_info[opcode].mask[qp->ibqp.qp_type];
>   }
> 
> --
> 2.43.0

Re: [PATCH v2] RDMA/rxe: bound the WQE opcode before indexing rxe_wr_opcode_info[]

[Youngsung Ahn]

> However, for req_retry(), it is less clear to me what the subsequent
> control flow is when wr_opcode_mask() returns 0. In particular, I would
> like to understand whether returning 0 for an out-of-range opcode is
> sufficient to safely handle the invalid WQE in this path.

I checked the code again. req_retry() does not reject the WQE. It only
re-posts it. The rejection happens later, on the next requester pass.

Of the three callers, only validate_send_wr() rejects a zero mask.
req_next_wqe() just stores the mask into wqe->mask.

In req_retry(), the mask is used only in "mask & WR_*" tests. So when the
mask is 0, all of them are false (abridged, 7.3-rc4):

	for (wqe_index = cons; wqe_index != prod;
			wqe_index = queue_next_index(q, wqe_index)) {
		wqe = queue_addr_from_index(qp->sq.queue, wqe_index);
		mask = wr_opcode_mask(wqe->wr.opcode, qp);

		if (wqe->state == wqe_state_posted)
			break;
		if (wqe->state == wqe_state_done)
			continue;

		wqe->iova = (mask & WR_ATOMIC_MASK) ?
			     wqe->wr.wr.atomic.remote_addr :
			     (mask & WR_READ_OR_WRITE_MASK) ?
			     wqe->wr.wr.rdma.remote_addr :
			     0;

		if (!first || (mask & WR_READ_MASK) == 0) {
			wqe->dma.resid = wqe->dma.length;
			wqe->dma.cur_sge = 0;
			wqe->dma.sge_offset = 0;
		}

		if (first) {
			first = 0;

			if (mask & WR_WRITE_OR_SEND_MASK) {
				...
				retry_first_write_send(qp, wqe, npsn);
			}

			if (mask & WR_READ_MASK) {
				...
			}
		}

		wqe->state = wqe_state_posted;
	}

So with mask 0 the WQE gets iova = 0, and the DMA cursor is reset to the
start. retry_first_write_send() and the read fix-up are both skipped. The
WQE stays in wqe_state_posted. With the bound in place, no array is
indexed here, so nothing from the ring is dereferenced. But the WQE is
still in the queue. It is not completed yet.

The next requester pass completes it with an error:

 - req_next_wqe() stores the same mask 0 into wqe->mask.
 - wqe->mask & WR_LOCAL_OP_MASK is false, so rxe_do_local_ops() is not
   called.
 - next_opcode() gets the raw wqe->wr.opcode. It matches no case in
   next_opcode_rc(), next_opcode_uc(), or the UD/GSI switch, so it
   returns -EINVAL.
 - rxe_requester() has this:

	opcode = next_opcode(qp, wqe, wqe->wr.opcode);
	if (unlikely(opcode < 0)) {
		wqe->status = IB_WC_LOC_QP_OP_ERR;
		goto err;
	}

   Then err: advances qp->req.wqe_index, sets wqe->state =
   wqe_state_error, and calls rxe_qp_error(). So the QP goes to
   IB_QPS_ERR, and the completer posts the completion. After my test run
   the ring shows state = 4 and status = 2.

So I think returning 0 is enough for this path. The invalid WQE never
reaches the code that uses the mask to choose an operation. It is
completed with a local QP operation error, not retried forever.

There are two exits before next_opcode(), but I do not think they change
this. rxe_wqe_is_fenced() can delay the WQE by one pass. If the QP is
already in IB_QPS_ERR, the WQE is flushed with IB_WC_WR_FLUSH_ERR. Neither
of them reads the array again.

req_retry() still needs the bound. wr_opcode_mask() is called at the top
of the loop body, before the wqe_state_posted and wqe_state_done checks.
So the out-of-bounds read happens even for WQEs that the loop skips.

About IB_WR_REG_MR instead of ARRAY_SIZE(): rxe_opcode.h only has
"extern struct rxe_wr_opcode_info rxe_wr_opcode_info[]", so the size is
not visible where wr_opcode_mask() is. IB_WR_REG_MR (32) is the highest
index that rxe_opcode.c initialises. On x86-64 the object is 1320 bytes
and the stride is 40 bytes, so there are 33 elements, and the valid
indices are 0 to 32.

> You mentioned that you were able to reproduce the issue on your local
> host. Would you be able to share the reproducer with us? It would help
> us better understand the failure path and verify the fix, especially for
> the req_retry() case.

The fix is not merged yet, so I am not posting the reproducer on the list.
I am sending it to you directly in a separate mail. It is a small C
program and it does not need libibverbs:

	gcc -O1 -o rxe_opcode_oob rxe_opcode_oob.c
	rxe_opcode_oob <opcode-as-u32>

It creates a user RC QP, mmaps the SQ, writes one rxe_send_wqe with
wr.opcode = <opcode> into the ring, and rings the legacy POST_SEND
doorbell. It memsets the WQE to 0 first, so wr.reg.mr is NULL and
wr.ex.invalidate_rkey is 0. Please use an opcode whose low byte is not
0x20 (IB_WR_REG_MR). Otherwise the run also reaches the rxe_reg_fast_mr()
problem, which is a different patch.

My results on 7.3-rc4 (93f51579e7df), KASAN x86-64, with the program run
as uid 1000:

 - KASAN global-out-of-bounds in rxe_requester, "Read of size 4" at
   rxe_wr_opcode_info+0x538. The splat shows the rxe_wq kworker, because
   the WQE is consumed there, and wr_opcode_mask() is inlined into
   rxe_requester().
 - For larger opcodes I get an Oops or a GPF instead. RIP is in
   wr_opcode_mask (rxe_loc.h:187) [inline], then req_next_wqe
   (rxe_req.c:196) [inline], then rxe_requester+0x66c or +0x68d.
 - The loaded mask is written back into the ring slot, so the program
   prints the value it read. For opcodes past the redzone (34, 36 and 174
   in my runs) it is a kernel dword. For 33 it reads 0, and the KASAN
   splat is the signal.

One limitation. This reproducer only drives req_next_wqe(). It does not
drive req_retry(). For that path the completer has to ask for a retry
first (rnr_nak_timer(), or the COMPST_RETRY path in rxe_comp.c). So what I
wrote about req_retry() above comes from reading the code, not from a
test. If it is useful, I can extend the reproducer to force the retry
path.

I will resend the patch as a standalone message, not as a reply to this
thread, as you and Leon said.

Youngsung Ahn

---8<--- KASAN evidence (7.3-rc4 93f51579e7df, CONFIG_KASAN=y, uid 1000) ---8<---

$ ./rxe_opcode_oob 33
=== opcode=33 (0x21)  uid=1000 ===
wqe @0x7ff84f128180  wr.opcode=0x21  pre-mask=0xdeadbeef
doorbell rung

[    5.487092] ==================================================================
[    5.487107] BUG: KASAN: global-out-of-bounds in rxe_requester+0x2a5/0x1d00
[    5.487170] Read of size 4 at addr ffffffff845513b8 by task kworker/u8:1/32
[    5.487184] CPU: 1 UID: 0 PID: 32 Comm: kworker/u8:1 Not tainted 7.3.0-rc4 #3
[    5.487201] Workqueue: rxe_wq do_work
[    5.487214] Call Trace:
[    5.487226]  dump_stack_lvl+0x53/0x70
[    5.487271]  print_report+0xd0/0x630
[    5.487387]  kasan_report+0xce/0x100
[    5.487416]  rxe_requester+0x2a5/0x1d00
[    5.487555]  rxe_sender+0xe/0x30
[    5.487565]  do_work+0xb6/0x250
[    5.487576]  process_one_work+0x3d1/0x790
[    5.487649]  worker_thread+0x296/0x500
[    5.487671]  kthread+0x194/0x1e0
[    5.487691]  ret_from_fork+0x2ac/0x3c0
[    5.487751]  ret_from_fork_asm+0x1a/0x30
[    5.487770] The buggy address belongs to the variable:
[    5.487772]  rxe_wr_opcode_info+0x538/0x19e0
[    5.487847] Memory state around the buggy address:
[    5.487851]  ffffffff84551280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[    5.487857]  ffffffff84551300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[    5.487863] >ffffffff84551380: 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
[    5.487868]                                         ^
[    5.487872]  ffffffff84551400: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[    5.487879]  ffffffff84551480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[    5.487883] ==================================================================
RESULT opcode=0x21  leaked_mask=0x00000000  state=4 status=2  sq(prod=1 cons=1) cq_prod=2

$ ./rxe_opcode_oob 34       # past the redzone: silent 4-byte leak, no splat
RESULT opcode=0x22  leaked_mask=0x83c15aa0  state=4 status=2  sq(prod=1 cons=1) cq_prod=2

$ ./rxe_opcode_oob 174
RESULT opcode=0xae  leaked_mask=0x8353d9c0  state=4 status=2  sq(prod=1 cons=1) cq_prod=2

rxe_wr_opcode_info is 1320 bytes (33 entries, 40-byte stride), so the valid
indices are 0..32. For opcode 33 the load is at

	base + 33*40 + offsetof(mask) + IB_QPT_RC*4 = base + 0x538

and ffffffff84550e80 + 0x538 = ffffffff845513b8, which is the address in the
splat -- one entry past the end. KASAN only catches the first entry past the
array (the f9 redzone above); opcodes 34 and 174 land in unpoisoned memory, so
they return a kernel dword silently and the mask is written back into the
user-visible ring slot, which is what the program prints.

Re: [PATCH v2] RDMA/rxe: bound the WQE opcode before indexing rxe_wr_opcode_info[]

[Zhu Yanjun]

在 2026/10/2 9:45, Youngsung Ahn 写道:
>> However, for req_retry(), it is less clear to me what the subsequent
>> control flow is when wr_opcode_mask() returns 0. In particular, I would
>> like to understand whether returning 0 for an out-of-range opcode is
>> sufficient to safely handle the invalid WQE in this path.
> 
> I checked the code again. req_retry() does not reject the WQE. It only
> re-posts it. The rejection happens later, on the next requester pass.
> 
> Of the three callers, only validate_send_wr() rejects a zero mask.
> req_next_wqe() just stores the mask into wqe->mask.
> 
> In req_retry(), the mask is used only in "mask & WR_*" tests. So when the
> mask is 0, all of them are false (abridged, 7.3-rc4):
> 
> 	for (wqe_index = cons; wqe_index != prod;
> 			wqe_index = queue_next_index(q, wqe_index)) {
> 		wqe = queue_addr_from_index(qp->sq.queue, wqe_index);
> 		mask = wr_opcode_mask(wqe->wr.opcode, qp);
> 
> 		if (wqe->state == wqe_state_posted)
> 			break;
> 		if (wqe->state == wqe_state_done)
> 			continue;
> 
> 		wqe->iova = (mask & WR_ATOMIC_MASK) ?
> 			     wqe->wr.wr.atomic.remote_addr :
> 			     (mask & WR_READ_OR_WRITE_MASK) ?
> 			     wqe->wr.wr.rdma.remote_addr :
> 			     0;
> 
> 		if (!first || (mask & WR_READ_MASK) == 0) {
> 			wqe->dma.resid = wqe->dma.length;
> 			wqe->dma.cur_sge = 0;
> 			wqe->dma.sge_offset = 0;
> 		}
> 
> 		if (first) {
> 			first = 0;
> 
> 			if (mask & WR_WRITE_OR_SEND_MASK) {
> 				...
> 				retry_first_write_send(qp, wqe, npsn);
> 			}
> 
> 			if (mask & WR_READ_MASK) {
> 				...
> 			}
> 		}
> 
> 		wqe->state = wqe_state_posted;
> 	}
> 
> So with mask 0 the WQE gets iova = 0, and the DMA cursor is reset to the
> start. retry_first_write_send() and the read fix-up are both skipped. The
> WQE stays in wqe_state_posted. With the bound in place, no array is
> indexed here, so nothing from the ring is dereferenced. But the WQE is
> still in the queue. It is not completed yet.
> 
> The next requester pass completes it with an error:
> 
>   - req_next_wqe() stores the same mask 0 into wqe->mask.
>   - wqe->mask & WR_LOCAL_OP_MASK is false, so rxe_do_local_ops() is not
>     called.
>   - next_opcode() gets the raw wqe->wr.opcode. It matches no case in
>     next_opcode_rc(), next_opcode_uc(), or the UD/GSI switch, so it
>     returns -EINVAL.
>   - rxe_requester() has this:
> 
> 	opcode = next_opcode(qp, wqe, wqe->wr.opcode);
> 	if (unlikely(opcode < 0)) {
> 		wqe->status = IB_WC_LOC_QP_OP_ERR;
> 		goto err;
> 	}
> 
>     Then err: advances qp->req.wqe_index, sets wqe->state =
>     wqe_state_error, and calls rxe_qp_error(). So the QP goes to
>     IB_QPS_ERR, and the completer posts the completion. After my test run
>     the ring shows state = 4 and status = 2.
> 
> So I think returning 0 is enough for this path. The invalid WQE never
> reaches the code that uses the mask to choose an operation. It is
> completed with a local QP operation error, not retried forever.
> 
> There are two exits before next_opcode(), but I do not think they change
> this. rxe_wqe_is_fenced() can delay the WQE by one pass. If the QP is
> already in IB_QPS_ERR, the WQE is flushed with IB_WC_WR_FLUSH_ERR. Neither
> of them reads the array again.
> 
> req_retry() still needs the bound. wr_opcode_mask() is called at the top
> of the loop body, before the wqe_state_posted and wqe_state_done checks.
> So the out-of-bounds read happens even for WQEs that the loop skips.
> 
> About IB_WR_REG_MR instead of ARRAY_SIZE(): rxe_opcode.h only has
> "extern struct rxe_wr_opcode_info rxe_wr_opcode_info[]", so the size is
> not visible where wr_opcode_mask() is. IB_WR_REG_MR (32) is the highest
> index that rxe_opcode.c initialises. On x86-64 the object is 1320 bytes
> and the stride is 40 bytes, so there are 33 elements, and the valid
> indices are 0 to 32.
> 
>> You mentioned that you were able to reproduce the issue on your local
>> host. Would you be able to share the reproducer with us? It would help
>> us better understand the failure path and verify the fix, especially for
>> the req_retry() case.
> 
> The fix is not merged yet, so I am not posting the reproducer on the list.
> I am sending it to you directly in a separate mail. It is a small C
> program and it does not need libibverbs:
> 
> 	gcc -O1 -o rxe_opcode_oob rxe_opcode_oob.c
> 	rxe_opcode_oob <opcode-as-u32>
> 
> It creates a user RC QP, mmaps the SQ, writes one rxe_send_wqe with
> wr.opcode = <opcode> into the ring, and rings the legacy POST_SEND
> doorbell. It memsets the WQE to 0 first, so wr.reg.mr is NULL and
> wr.ex.invalidate_rkey is 0. Please use an opcode whose low byte is not
> 0x20 (IB_WR_REG_MR). Otherwise the run also reaches the rxe_reg_fast_mr()
> problem, which is a different patch.
> 
> My results on 7.3-rc4 (93f51579e7df), KASAN x86-64, with the program run
> as uid 1000:
> 
>   - KASAN global-out-of-bounds in rxe_requester, "Read of size 4" at
>     rxe_wr_opcode_info+0x538. The splat shows the rxe_wq kworker, because
>     the WQE is consumed there, and wr_opcode_mask() is inlined into
>     rxe_requester().
>   - For larger opcodes I get an Oops or a GPF instead. RIP is in
>     wr_opcode_mask (rxe_loc.h:187) [inline], then req_next_wqe
>     (rxe_req.c:196) [inline], then rxe_requester+0x66c or +0x68d.
>   - The loaded mask is written back into the ring slot, so the program
>     prints the value it read. For opcodes past the redzone (34, 36 and 174
>     in my runs) it is a kernel dword. For 33 it reads 0, and the KASAN
>     splat is the signal.
> 
> One limitation. This reproducer only drives req_next_wqe(). It does not
> drive req_retry(). For that path the completer has to ask for a retry
> first (rnr_nak_timer(), or the COMPST_RETRY path in rxe_comp.c). So what I
> wrote about req_retry() above comes from reading the code, not from a
> test. If it is useful, I can extend the reproducer to force the retry
> path.
> 
> I will resend the patch as a standalone message, not as a reply to this
> thread, as you and Leon said.

Great.

Please send the patch as a standalone commit. For the latest commit, 
please also include the test results.

If the reproducer is not suitable for posting to the mailing list, 
please send it to me offline so that I can verify the commit on my local 
system.

The same applies to all the commits you have sent.

Thanks
Yanjun Zhu

> 
> Youngsung Ahn
> 
> ---8<--- KASAN evidence (7.3-rc4 93f51579e7df, CONFIG_KASAN=y, uid 1000) ---8<---
> 
> $ ./rxe_opcode_oob 33
> === opcode=33 (0x21)  uid=1000 ===
> wqe @0x7ff84f128180  wr.opcode=0x21  pre-mask=0xdeadbeef
> doorbell rung
> 
> [    5.487092] ==================================================================
> [    5.487107] BUG: KASAN: global-out-of-bounds in rxe_requester+0x2a5/0x1d00
> [    5.487170] Read of size 4 at addr ffffffff845513b8 by task kworker/u8:1/32
> [    5.487184] CPU: 1 UID: 0 PID: 32 Comm: kworker/u8:1 Not tainted 7.3.0-rc4 #3
> [    5.487201] Workqueue: rxe_wq do_work
> [    5.487214] Call Trace:
> [    5.487226]  dump_stack_lvl+0x53/0x70
> [    5.487271]  print_report+0xd0/0x630
> [    5.487387]  kasan_report+0xce/0x100
> [    5.487416]  rxe_requester+0x2a5/0x1d00
> [    5.487555]  rxe_sender+0xe/0x30
> [    5.487565]  do_work+0xb6/0x250
> [    5.487576]  process_one_work+0x3d1/0x790
> [    5.487649]  worker_thread+0x296/0x500
> [    5.487671]  kthread+0x194/0x1e0
> [    5.487691]  ret_from_fork+0x2ac/0x3c0
> [    5.487751]  ret_from_fork_asm+0x1a/0x30
> [    5.487770] The buggy address belongs to the variable:
> [    5.487772]  rxe_wr_opcode_info+0x538/0x19e0
> [    5.487847] Memory state around the buggy address:
> [    5.487851]  ffffffff84551280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [    5.487857]  ffffffff84551300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [    5.487863] >ffffffff84551380: 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
> [    5.487868]                                         ^
> [    5.487872]  ffffffff84551400: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [    5.487879]  ffffffff84551480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [    5.487883] ==================================================================
> RESULT opcode=0x21  leaked_mask=0x00000000  state=4 status=2  sq(prod=1 cons=1) cq_prod=2
> 
> $ ./rxe_opcode_oob 34       # past the redzone: silent 4-byte leak, no splat
> RESULT opcode=0x22  leaked_mask=0x83c15aa0  state=4 status=2  sq(prod=1 cons=1) cq_prod=2
> 
> $ ./rxe_opcode_oob 174
> RESULT opcode=0xae  leaked_mask=0x8353d9c0  state=4 status=2  sq(prod=1 cons=1) cq_prod=2
> 
> rxe_wr_opcode_info is 1320 bytes (33 entries, 40-byte stride), so the valid
> indices are 0..32. For opcode 33 the load is at
> 
> 	base + 33*40 + offsetof(mask) + IB_QPT_RC*4 = base + 0x538
> 
> and ffffffff84550e80 + 0x538 = ffffffff845513b8, which is the address in the
> splat -- one entry past the end. KASAN only catches the first entry past the
> array (the f9 redzone above); opcodes 34 and 174 land in unpoisoned memory, so
> they return a kernel dword silently and the mask is written back into the
> user-visible ring slot, which is what the program prints.