[PATCH net] net/smc: free pending qentry in smc_llc_flow_stop() before memset

[PATCH net] net/smc: free pending qentry in smc_llc_flow_stop() before memset

[Mahanta Jambigi]

smc_llc_flow_stop() resets a flow struct with a blind memset:

	spin_lock_bh(&lgr->llc_flow_lock);
	memset(flow, 0, sizeof(*flow));
	flow->type = SMC_LLC_FLOW_NONE;
	spin_unlock_bh(&lgr->llc_flow_lock);

If flow->qentry is non-NULL at this point the pointer is overwritten without the
allocation being freed, leaking one kmalloc object.

A late-arriving duplicate CONFIRM_LINK or ADD_LINK_CONT message can set
flow->qentry after the legitimate message has been consumed by the waiter via
smc_llc_flow_qentry_clr() (which NULLs the pointer but leaves flow->type
non-zero) but before the flow completes and smc_llc_flow_stop() runs.  In that
window the duplicate is stashed into flow->qentry, and then lost when
smc_llc_flow_stop() zeros the struct.

Call smc_llc_flow_qentry_del() inside the lock before the memset.
smc_llc_flow_qentry_del() already checks flow->qentry before freeing, so the
normal case where no entry is pending is a no-op.

Fixes: 555da9af827d ("net/smc: add event-based llc_flow framework")
Reviewed-by: Hidayath Khan <hidayath@linux.ibm.com>
Signed-off-by: Mahanta Jambigi <mjambigi@linux.ibm.com>
---
 net/smc/smc_llc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 954b2ff1815c..fa1ff543638f 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -320,6 +320,7 @@ int smc_llc_flow_initiate(struct smc_link_group *lgr,
 void smc_llc_flow_stop(struct smc_link_group *lgr, struct smc_llc_flow *flow)
 {
 	spin_lock_bh(&lgr->llc_flow_lock);
+	smc_llc_flow_qentry_del(flow);
 	memset(flow, 0, sizeof(*flow));
 	flow->type = SMC_LLC_FLOW_NONE;
 	spin_unlock_bh(&lgr->llc_flow_lock);
-- 
2.50.1 (Apple Git-155)

Re: [PATCH net] net/smc: free pending qentry in smc_llc_flow_stop() before memset

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix for memory leak of late-arriving LLC messages due to a TOCTOU data race.
- [High] Preexisting memory leak in `smc_llc_lgr_clear()` where canceled asynchronous flow tasks leak dynamically allocated `qentry` objects.
--

--- Patch [1]: [PATCH net] net/smc: free pending qentry in smc_llc_flow_stop() before memset ---
commit 4bed80c54874ec213bfc62c4cef2f581d5c51aa6
Author: Mahanta Jambigi <mjambigi@linux.ibm.com>

net/smc: free pending qentry in smc_llc_flow_stop() before memset

This commit addresses a memory leak by freeing the flow->qentry pointer inside
smc_llc_flow_stop() before the flow structure is cleared with memset.

> diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
> --- a/net/smc/smc_llc.c
> +++ b/net/smc/smc_llc.c
> @@ -320,6 +320,7 @@ int smc_llc_flow_initiate(struct smc_link_group *lgr,
>  void smc_llc_flow_stop(struct smc_link_group *lgr, struct smc_llc_flow *flow)
>  {
>  	spin_lock_bh(&lgr->llc_flow_lock);
> +	smc_llc_flow_qentry_del(flow);
>  	memset(flow, 0, sizeof(*flow));

[Severity: High]
This isn't a bug introduced by this patch, but does this fix fully resolve
the leak?

Looking at smc_llc_rx_response() and smc_llc_event_handler(), they appear
to read flow->type and write to flow->qentry locklessly:

net/smc/smc_llc.c:smc_llc_rx_response() {
    ...
    /* assign responses to the local flow, we requested them */
    smc_llc_flow_qentry_set(&link->lgr->llc_flow_lcl, qentry);
    ...
}

Could a remote peer sending duplicate or late-arriving messages cause
smc_llc_rx_response() on one CPU to locklessly assign a new qentry to
flow->qentry either during or immediately after the memset() executes here?

If that happens, wouldn't the newly assigned qentry either be zeroed out
or left dangling and overwritten when the next flow starts, resulting in
another leak?

>  	flow->type = SMC_LLC_FLOW_NONE;
>  	spin_unlock_bh(&lgr->llc_flow_lock);

[Severity: High]
This is a pre-existing issue, but does smc_llc_lgr_clear() also leak these
dynamically allocated qentry objects?

When an LLC flow starts, a qentry is stored in lgr->llc_flow_lcl.qentry or
lgr->llc_flow_rmt.qentry. If the link group is suddenly terminated,
smc_llc_lgr_clear() cancels the background tasks:

net/smc/smc_llc.c:smc_llc_lgr_clear() {
    ...
    cancel_work_sync(&lgr->llc_event_work);
    cancel_work_sync(&lgr->llc_add_link_work);
    cancel_work_sync(&lgr->llc_del_link_work);
    if (lgr->delayed_event) {
        kfree(lgr->delayed_event);
        lgr->delayed_event = NULL;
    }
    ...
}

Since these canceled works never process or free the stored qentries, and
smc_llc_lgr_clear() only frees lgr->delayed_event, will
lgr->llc_flow_lcl.qentry and lgr->llc_flow_rmt.qentry be permanently leaked
when the link group is freed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818073943.1108383-1-mjambigi@linux.ibm.com?part=1

Re: [PATCH net] net/smc: free pending qentry in smc_llc_flow_stop() before memset

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 18 Aug 2026 09:39:43 +0200 you wrote:
> smc_llc_flow_stop() resets a flow struct with a blind memset:
> 
> 	spin_lock_bh(&lgr->llc_flow_lock);
> 	memset(flow, 0, sizeof(*flow));
> 	flow->type = SMC_LLC_FLOW_NONE;
> 	spin_unlock_bh(&lgr->llc_flow_lock);
> 
> [...]

Here is the summary with links:
  - [net] net/smc: free pending qentry in smc_llc_flow_stop() before memset
    https://git.kernel.org/netdev/net/c/5ee0ceddc778

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html