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:
 <CH2PPF910B3338DACDB788F3574AFADCFE4CA8F2@CH2PPF910B3338D.namprd21.prod.outlook.com>
Date: Thu, 22 Aug 2024 14:30:24 +0000
From: Haiyang Zhang <haiyangz@...rosoft.com>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>
CC: "ast@...nel.org" <ast@...nel.org>, "bpf@...r.kernel.org"
	<bpf@...r.kernel.org>, "daniel@...earbox.net" <daniel@...earbox.net>,
	"davem@...emloft.net" <davem@...emloft.net>, Dexuan Cui
	<decui@...rosoft.com>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"hawk@...nel.org" <hawk@...nel.org>, "jesse.brandeburg@...el.com"
	<jesse.brandeburg@...el.com>, "john.fastabend@...il.com"
	<john.fastabend@...il.com>, "kuba@...nel.org" <kuba@...nel.org>, KY
 Srinivasan <kys@...rosoft.com>, "leon@...nel.org" <leon@...nel.org>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>, Long Li
	<longli@...rosoft.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"olaf@...fle.de" <olaf@...fle.de>, "pabeni@...hat.com" <pabeni@...hat.com>,
	Paul Rosswurm <paulros@...rosoft.com>, "shradhagupta@...ux.microsoft.com"
	<shradhagupta@...ux.microsoft.com>, "ssengar@...ux.microsoft.com"
	<ssengar@...ux.microsoft.com>, "stable@...r.kernel.org"
	<stable@...r.kernel.org>, "stephen@...workplumber.org"
	<stephen@...workplumber.org>, "tglx@...utronix.de" <tglx@...utronix.de>,
	"vkuznets@...hat.com" <vkuznets@...hat.com>, "wei.liu@...nel.org"
	<wei.liu@...nel.org>
Subject: RE: [PATCH net] net: mana: Fix race of mana_hwc_post_rx_wqe and new
 hwc response



> -----Original Message-----
> From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
> Sent: Thursday, August 22, 2024 1:34 AM
> To: Haiyang Zhang <haiyangz@...rosoft.com>
> Cc: ast@...nel.org; bpf@...r.kernel.org; daniel@...earbox.net;
> davem@...emloft.net; Dexuan Cui <decui@...rosoft.com>;
> edumazet@...gle.com; hawk@...nel.org; jesse.brandeburg@...el.com;
> john.fastabend@...il.com; kuba@...nel.org; KY Srinivasan
> <kys@...rosoft.com>; leon@...nel.org; linux-hyperv@...r.kernel.org; linux-
> kernel@...r.kernel.org; linux-rdma@...r.kernel.org; Long Li
> <longli@...rosoft.com>; netdev@...r.kernel.org; olaf@...fle.de;
> pabeni@...hat.com; Paul Rosswurm <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?

Agreed. This patch just moves the function for the bug fix.
We will do code cleanups in other patches.

Thanks,
- Haiyang