Re: [PATCH v5 07/15] interconnect: qcom: Add Kuno interconnect provider driver

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Dangling pointers in statically allocated `qcom_icc_node` structs cause Use-After-Free on driver rebind or deferred probe.
--

commit 247254381db99e8828d00f4e780472baac9636d1
Author: Hardeep Sharma <hardeep.sharma@oss.qualcomm.com>

interconnect: qcom: Add Kuno interconnect provider driver
    
This commit adds a new interconnect provider driver for the Qualcomm Kuno
SoC. It defines the interconnect nodes and BCM structures, and registers
the interconnect provider using the common RPMh probe and remove functions.

> diff --git a/drivers/interconnect/qcom/kuno.c b/drivers/interconnect/qcom/kuno.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/interconnect/qcom/kuno.c
> @@ -0,0 +1,988 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/interconnect.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include <dt-bindings/interconnect/qcom,kuno.h>
> +
> +#include "bcm-voter.h"
> +#include "icc-rpmh.h"
> +
> +static struct qcom_icc_node qup0_core_slave = {
> +	.name = "qup0_core_slave",
> +	.channels = 1,
> +	.buswidth = 4,
> +	.num_links = 0,
> +};

[Severity: High]
This isn't a bug introduced by this patch, but does the use of statically
allocated nodes like qup0_core_slave in kuno.c create a risk of a dangling
pointer?

When the driver binds, qcom_icc_rpmh_probe() allocates dynamic icc_node
structures and caches them in the node field of these static definitions.

On driver unbind or deferred probe error paths, qcom_icc_rpmh_remove()
frees those structures via kfree(). However, the static pointers in the
qcom_icc_node arrays are never cleared to NULL.

If a privileged user unbinds and rebinds the driver via sysfs, or if a
deferred probe is triggered on startup, could the new probe call bypass the
if (!qn->node) check, reuse the freed pointer in icc_node_set_name() or
icc_node_add(), and result in memory corruption or a use-after-free?

[ ... ]

> +static struct platform_driver qnoc_driver = {
> +	.probe = qcom_icc_rpmh_probe,
> +	.remove = qcom_icc_rpmh_remove,
> +	.driver = {
> +		.name = "qnoc-kuno",
> +		.of_match_table = qnoc_of_match,
> +		.sync_state = icc_sync_state,
> +	},
> +};

[Severity: High]
This isn't a bug introduced by this patch, but since qnoc_driver registers
with qcom_icc_rpmh_probe and qcom_icc_rpmh_remove, does it inherit the
unbind and rebind issue described above?

Because qcom_icc_rpmh_remove() does not clear the cached pointers from the
static node arrays provided by this driver, could a deferred probe or sysfs
rebind result in dereferencing freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-kuno-soc-support-v5-0-6d47636a8f09@oss.qualcomm.com?part=7