Re: [PATCH 5.15] net/smc: reject CHID-0 ACCEPT that matches an empty ism_dev slot

[email protected]
Newsgroups org.kernel.vger.linux-s390
Message-ID <[email protected]>
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 <[email protected]>

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/[email protected]
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/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.