netdev - Re: [PATCH net] net: mana: Fix race of mana_hwc_post_rx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1fe20e5e-1c90-4029-9d40-625ec3bb3248@wanadoo.fr>
Date: Thu, 22 Aug 2024 07:34:16 +0200
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Haiyang Zhang <haiyangz@...rosoft.com>
Cc: ast@...nel.org, bpf@...r.kernel.org, daniel@...earbox.net,
 davem@...emloft.net, decui@...rosoft.com, edumazet@...gle.com,
 hawk@...nel.org, jesse.brandeburg@...el.com, john.fastabend@...il.com,
 kuba@...nel.org, kys@...rosoft.com, leon@...nel.org,
 linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-rdma@...r.kernel.org, longli@...rosoft.com, netdev@...r.kernel.org,
 olaf@...fle.de, pabeni@...hat.com, paulros@...rosoft.com,
 shradhagupta@...ux.microsoft.com, ssengar@...ux.microsoft.com,
 stable@...r.kernel.org, stephen@...workplumber.org, tglx@...utronix.de,
 vkuznets@...hat.com, wei.liu@...nel.org
Subject: Re: [PATCH net] net: mana: Fix race of mana_hwc_post_rx_wqe and new
 hwc response

Le 21/08/2024 à 22:42, Haiyang Zhang a écrit :
> The mana_hwc_rx_event_handler() / mana_hwc_handle_resp() calls
> complete(&ctx->comp_event) before posting the wqe back. It's
> possible that other callers, like mana_create_txq(), start the
> next round of mana_hwc_send_request() before the posting of wqe.
> And if the HW is fast enough to respond, it can hit no_wqe error
> on the HW channel, then the response message is lost. The mana
> driver may fail to create queues and open, because of waiting for
> the HW response and timed out.
> Sample dmesg:
> [  528.610840] mana 39d4:00:02.0: HWC: Request timed out!
> [  528.614452] mana 39d4:00:02.0: Failed to send mana message: -110, 0x0
> [  528.618326] mana 39d4:00:02.0 enP14804s2: Failed to create WQ object: -110
> 
> To fix it, move posting of rx wqe before complete(&ctx->comp_event).
> 
> Cc: stable-u79uwXL29TY76Z2rM5mHXA@...lic.gmane.org
> Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
> Signed-off-by: Haiyang Zhang <haiyangz-0li6OtcxBFHby3iVrkZq2A@...lic.gmane.org>
> ---
>   .../net/ethernet/microsoft/mana/hw_channel.c  | 62 ++++++++++---------
>   1 file changed, 34 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index cafded2f9382..a00f915c5188 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> @@ -52,9 +52,33 @@ static int mana_hwc_verify_resp_msg(const struct hwc_caller_ctx *caller_ctx,
>   	return 0;
>   }
>   
> +static int mana_hwc_post_rx_wqe(const struct hwc_wq *hwc_rxq,
> +				struct hwc_work_request *req)
> +{
> +	struct device *dev = hwc_rxq->hwc->dev;
> +	struct gdma_sge *sge;
> +	int err;
> +
> +	sge = &req->sge;
> +	sge->address = (u64)req->buf_sge_addr;
> +	sge->mem_key = hwc_rxq->msg_buf->gpa_mkey;
> +	sge->size = req->buf_len;
> +
> +	memset(&req->wqe_req, 0, sizeof(struct gdma_wqe_request));
> +	req->wqe_req.sgl = sge;
> +	req->wqe_req.num_sge = 1;
> +	req->wqe_req.client_data_unit = 0;

Hi,

unrelated to your patch, but this initialization is useless, it is 
already memset(0)'ed a few lines above.
So why client_data_unit and not some other fields?

> +
> +	err = mana_gd_post_and_ring(hwc_rxq->gdma_wq, &req->wqe_req, NULL);
> +	if (err)
> +		dev_err(dev, "Failed to post WQE on HWC RQ: %d\n", err);
> +	return err;
> +}

...

Just my 2c.

CJ