Re: [PATCH net] net/smc: fix qentry overwrite for CONFIRM_LINK and ADD_LINK_CONT in smc_llc_event_handler()
Dust Li <[email protected]>
| Newsgroups | org.kernel.vger.linux-s390,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
On 2026-07-29 15:01:53, Mahanta Jambigi wrote:
>The SMC_LLC_CONFIRM_LINK / SMC_LLC_ADD_LINK_CONT branch in
>smc_llc_event_handler() stores an incoming qentry into the local LLC flow
>without first checking whether a qentry is already pending. If a malicious or
>buggy peer sends a second CONFIRM_LINK or ADD_LINK_CONT request while a flow is
>active and flow->qentry is already set, smc_llc_flow_qentry_set() overwrites the
>pointer without freeing the previous allocation, leaking one kmalloc-96 object
>per spurious message.
>
>The sibling SMC_LLC_DELETE_LINK branch already has the correct !flow->qentry
>guard. Apply the same guard to the CONFIRM_LINK/ADD_LINK_CONT branch so that a
>duplicate message when qentry is already occupied falls through to break and is
>freed by the kfree(qentry) at the out: label, rather than silently leaking the
>existing allocation.
>
>The response direction (smc_llc_rx_response()) is unaffected: it already guards
>with flow->qentry at the equivalent site and drops duplicate responses
>correctly.
>
>Fixes: 0fb0b02bd6fd ("net/smc: adapt SMC client code to use the LLC flow")
>Signed-off-by: Mahanta Jambigi <[email protected]>
>Reviewed-by: Hidayath Khan <[email protected]>
>Reviewed-by: Sidraya Jayagond <[email protected]>
>---
> net/smc/smc_llc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
>index 954b2ff1815c..aa6d83af55ed 100644
>--- a/net/smc/smc_llc.c
>+++ b/net/smc/smc_llc.c
>@@ -1927,7 +1927,8 @@ static void smc_llc_event_handler(struct smc_llc_qentry *qentry)
> return;
> case SMC_LLC_CONFIRM_LINK:
> case SMC_LLC_ADD_LINK_CONT:
>- if (lgr->llc_flow_lcl.type != SMC_LLC_FLOW_NONE) {
>+ if (lgr->llc_flow_lcl.type != SMC_LLC_FLOW_NONE &&
>+ !lgr->llc_flow_lcl.qentry) {
I read sashiko's review comments, and I think both of them make sense, but
they were pre-existing issues. So I think we can solve them in the future
patches.
https://sashiko.dev/#/patchset/[email protected]
Reviewed-by: Dust Li <[email protected]>
Best regards,
Dust