linux-hardening - Re: [PATCH net-next v7 4/5] net: rnpgbe: Add basic mbx

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 for Android: free password hash cracker in your pocket

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

Message-ID: <05B2D818DB1348E6+20250827014211.GA469112@nic-Precision-5820-Tower>
Date: Wed, 27 Aug 2025 09:42:11 +0800
From: Yibo Dong <dong100@...se.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Vadim Fedorenko <vadim.fedorenko@...ux.dev>, andrew+netdev@...n.ch,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, horms@...nel.org, corbet@....net,
	gur.stavi@...wei.com, maddy@...ux.ibm.com, mpe@...erman.id.au,
	danishanwar@...com, lee@...ger.us, gongfan1@...wei.com,
	lorenzo@...nel.org, geert+renesas@...der.be,
	Parthiban.Veerasooran@...rochip.com, lukas.bulwahn@...hat.com,
	alexanderduyck@...com, richardcochran@...il.com, kees@...nel.org,
	gustavoars@...nel.org, netdev@...r.kernel.org,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-hardening@...r.kernel.org
Subject: Re: [PATCH net-next v7 4/5] net: rnpgbe: Add basic mbx_fw support

On Tue, Aug 26, 2025 at 02:39:07PM +0200, Andrew Lunn wrote:
> > Yes. It is not safe, so I 'must wait_event_timeout before free cookie'....
> > But is there a safe way to do it?
> > Maybe:
> > ->allocate cookie
> >   -> map it to an unique id
> >     ->set the id to req->cookie
> >       ->receive response and check id valid? Then access cookie?
> 
> This is part of why adding cookies in a separate patch with a good
> commit message is important.
> 
> Please take a step back. What is the big picture? Why do you need a
> cookie? What is it used for? If you describe what your requirements
> are, we might be able to suggest a better solution, or point you at a
> driver you can copy code from.
> 
> 	Andrew
> 

I try to explain the it:

driver-->fw, we has two types request:
1. without response, such as mucse_mbx_ifinsmod
2. with response, such as mucse_fw_get_macaddr

fw --> driver, we has one types request:
1. link status (link speed, duplex, pause status...)

fw tiggers irq when it sends response or request.
In order to handle link status timely, we do an irqhandle like this:

static int rnpgbe_rcv_msg_from_fw(struct mucse *mucse)
{
        u32 msgbuf[MUCSE_FW_MAILBOX_WORDS];
        struct mucse_hw *hw = &mucse->hw;
        struct mbx_fw_cmd_reply *reply;
        int retval;
	/* read mbx data out */
        retval = mucse_read_mbx(hw, msgbuf, MUCSE_FW_MAILBOX_WORDS);
        if (retval)
                return retval;

        reply = (struct mbx_fw_cmd_reply *)msgbuf;
	/* judge request or response */
        if (le16_to_cpu(reply->flags) & FLAGS_DD) {
		/* if it is a response, call wake_up(cookie) */
                return rnpgbe_mbx_fw_reply_handler(mucse,
                                (struct mbx_fw_cmd_reply *)msgbuf);
        } else {
		/* if it is a request, handle link status */
                return rnpgbe_mbx_fw_req_handler(mucse,
                                (struct mbx_fw_cmd_req *)msgbuf);
        }
}

And driver requests with response is bellow 'without' irqhandle:

static int mucse_fw_send_cmd_wait(struct mucse_hw *hw,
				  struct mbx_fw_cmd_req *req,
				  struct mbx_fw_cmd_reply *reply)
{
...
	mucse_write_posted_mbx(hw, (u32 *)req, len);

	...
	/* but as irqhandle be added, mbx data is read out in the
	 * handler, mucse_read_posted_mbx cannot read anything */
	mucse_read_posted_mbx(hw, (u32 *)reply, sizeof(*reply));

}

To solve mucse_read_posted_mbx cannot read data with irq, we add 'cookie'.
After mucse_write_posted_mbx, call wait_event_timeout. wake_up is called
in irqhandle.

Thanks for your feedback.