Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] Race condition leading to stack memory corruption and Use-After-Free (ABA) on hardware command timeout. -- --- Patch [4]: [PATCH net v4 4/7] net: mana: validate hardware-supplied values in the HWC RX path --- commit 3dd532e8ebcefe4a1f19044be675c58a1504e9c6 Author: Long Li <[email protected]> net: mana: validate hardware-supplied values in the HWC RX path This commit adds validation for hardware-supplied lengths and indices in the MANA hardware channel RX handler. It ensures the inline OOB size, SGE addresses, and response lengths are checked before use. Out-of-bounds or untrusted RX WQEs are leaked and, if exhausted, the channel goes into a terminal state. > diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c > b/drivers/net/ethernet/microsoft/mana/hw_channel.c > index 19896bb5ce1a4..5db8cfe2d8443 100644 > --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c > +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c > @@ -83,6 +83,19 @@ static void mana_hwc_handle_resp(struct hw_channel_context > *hwc, u32 resp_len, > struct hwc_caller_ctx *ctx; > int err; > > + /* Defence in depth: the sole caller, mana_hwc_rx_event_handler(), > + * already rejects msg_id >= hwc->num_inflight_msg with the value it > + * passes here by value, so this cannot be reached out of range. Keep > + * the guard at the indexing site so the bitmap and caller_ctx array > + * are never indexed without a bound in view. > + */ > + if (msg_id >= hwc->num_inflight_msg) { > + dev_err(hwc->dev, "hwc_rx: msg_id %u >= max %u\n", > + msg_id, hwc->num_inflight_msg); > + mana_hwc_post_rx_wqe(hwc->rxq, rx_req); > + return; > + } > + > if (!test_bit(msg_id, hwc->inflight_msg_res.map)) { > dev_err(hwc->dev, "hwc_rx: invalid msg_id = %u\n", msg_id); > mana_hwc_post_rx_wqe(hwc->rxq, rx_req); [Severity: Critical] This is a pre-existing issue, but is there a race condition between this test_bit() check and the command timeout path that could corrupt stack memory? If a thread calling mana_hwc_send_request() times out, it takes this path: if (!wait_for_completion_timeout(&ctx->comp_event, (hwc->hwc_timeout * HZ) / 1000)) { dev_err(hwc->dev, "HWC: Request timed out!\n"); err = -ETIMEDOUT; goto out; } ... out: mana_hwc_put_msg_index(hwc, msg_id); return err; The timeout path clears the message ID bit and returns without synchronizing with the interrupt handler. Concurrently, if a delayed hardware response arrives here in mana_hwc_handle_resp(), it could pass the test_bit() check right before the timeout path clears it. The timeout thread then clears the bit and returns, popping its stack. Then mana_hwc_handle_resp() proceeds and executes: if (!err && ctx->output_buf) { ctx->status_code = resp_msg->status; memcpy(ctx->output_buf, resp_msg, resp_len); Since ctx->output_buf points to the now-freed stack frame, could this memcpy() write hardware data into a freed stack frame or to the stack frame of a new thread reusing the message ID? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
