From: sashiko-bot@kernel.org
To: "Andrey Troshin" <drtrosh@yandex-team.ru>
Cc: "Heiko Carstens" <hca@linux.ibm.com>,
"Christian Borntraeger" <borntraeger@linux.ibm.com>,
"Alexander Gordeev" <agordeev@linux.ibm.com>,
linux-s390@vger.kernel.org, "Vasily Gorbik" <gor@linux.ibm.com>
Subject: Re: [PATCH 5.15] net/smc: reject CHID-0 ACCEPT that matches an empty ism_dev slot
Date: Fri, 21 Aug 2026 11:04:01 +0000 [thread overview]
Message-ID: <20260821110401.F3FEE1F00A3F@smtp.kernel.org> (raw)
In-Reply-To: <20260820110310.2090-1-drtrosh@yandex-team.ru>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] Unvalidated offsets and lengths in SMC CLC proposals allow arbitrary out-of-bounds heap memory reads.
- [High] Use-after-free of `smcd_dev` and `smc_ib_device` across a sleep due to missing reference tracking.
- [High] Unauthenticated NULL pointer dereference when processing SMC-D CLC proposals with missing v2 extensions.
- [High] Remote heap buffer overflow in `ini->ism_dev` array during ISM device matching.
--
--- Patch [1]: [PATCH 5.15] net/smc: reject CHID-0 ACCEPT that matches an empty ism_dev slot ---
commit 1aa51e986bc9d6f2f903ebb0f760865b75c754f4
Author: Xiang Mei <xmei5@asu.edu>
net/smc: reject CHID-0 ACCEPT that matches an empty ism_dev slot
On the SMC-D client, slot 0 of ini->ism_dev[]/ini->ism_chid[] is
reserved for an SMC-Dv1 device. smc_find_ism_v2_device_clnt()
populates V2 entries starting at index 1, so when no V1 device is
selected slot 0 is left in its kzalloc()'ed state.
Link: https://patch.msgid.link/20260511062138.2839584-1-xmei5@asu.edu
Link: https://nvd.nist.gov/vuln/detail/CVE-2026-64048
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index 78b663dbfa1f9..dd2a338eb6efe 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -869,7 +869,8 @@ smc_v2_determine_accepted_chid(struct smc_clc_msg_accept_confirm_v2 *aclc,
[ ... ]
> - if (ini->ism_chid[i] == ntohs(aclc->chid)) {
> + if (ini->ism_dev[i] &&
> + ini->ism_chid[i] == ntohs(aclc->chid)) {
> ini->ism_selected = i;
> return 0;
> }
> net/smc/smc_clc.c:smc_clc_msg_prop_valid()
> (hdr->typev2 != SMC_TYPE_N ?
> sizeof(*v2_ext) +
> v2_ext->hdr.eid_cnt * SMC_MAX_EID_LEN : 0) +
[Severity: High]
This isn't a bug introduced by this patch, but does this pointer calculation
validate that the offset is within the bounds of the allocated buffer?
It appears that in smc_get_clc_v2_ext(), the pointer is constructed using
an offset without bounds checking:
> net/smc/smc_clc.h:smc_get_clc_v2_ext()
> return (struct smc_clc_v2_extension *)
> ((u8 *)prop_smcd +
> offsetof(struct smc_clc_msg_smcd, v2_ext_offset) +
> sizeof(prop_smcd->v2_ext_offset) +
> ntohs(prop_smcd->v2_ext_offset));
Could this read beyond the heap allocation if the offset is too large?
[Severity: High]
This is a pre-existing issue, but if v2_ext_offset is explicitly 0,
smc_get_clc_v2_ext() returns NULL. Does this code safely handle a NULL
return for v2_ext? It looks like v2_ext->hdr.eid_cnt is unconditionally
dereferenced when hdr->typev2 indicates an SMC-D proposal.
> net/smc/af_smc.c:smc_find_ism_v2_device_clnt()
> ini->ism_dev[i] = smcd;
[Severity: High]
This isn't a bug introduced by this patch, but is it possible for the device
to be unregistered and freed during a sleep in smc_clc_wait_msg()?
The device pointer is stored without taking a reference count to pin its
lifetime. Later, the potentially freed device pointer is dereferenced after
waking up:
> net/smc/smc_core.c:smc_conn_create()
> lgr_list = ini->is_smcd ? &ini->ism_dev[ini->ism_selected]->lgr_list :
> &smc_lgr_list.list;
Would acquiring a reference count prevent a dangling pointer here?
> net/smc/af_smc.c:smc_find_ism_v2_device_serv()
> for (i = 1; i <= smc_v2_ext->hdr.ism_gid_cnt; i++) {
> smc_check_ism_v2_match(ini,
> ntohs(smcd_v2_ext->gidchid[i - 1].chid),
> ntohll(smcd_v2_ext->gidchid[i - 1].gid),
> &matches);
> }
[Severity: High]
This is a pre-existing issue, but is there a bounds check preventing matches
from exceeding the size of the ini->ism_dev array?
The loop executes based on ism_gid_cnt, and each unique match increments
matches and stores the device pointer:
> net/smc/af_smc.c:smc_check_ism_v2_match()
> ini->ism_peer_gid[*matches] = proposed_gid;
> ini->ism_dev[*matches] = smcd;
> (*matches)++;
Could this write past the end of the array if there are many ISM devices?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260820110310.2090-1-drtrosh@yandex-team.ru?part=1
prev parent reply other threads:[~2026-08-21 11:04 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-20 11:03 [PATCH 5.15] net/smc: reject CHID-0 ACCEPT that matches an empty ism_dev slot Andrey Troshin
2026-08-21 11:04 ` sashiko-bot [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260821110401.F3FEE1F00A3F@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=agordeev@linux.ibm.com \
--cc=borntraeger@linux.ibm.com \
--cc=drtrosh@yandex-team.ru \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=linux-s390@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.