Re: [PATCH v2 4/7] firmware: riscv: Add RPMI performance service

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] firmware: riscv: Silent truncation of 64-bit physical addresses on 32-bit platforms
- [High] firmware: riscv: UAF on manual unbind of RPMI performance driver
--

commit 3fe9e4964a54e548bf390b9011dab852e2cb26bb
Author: Joshua Yeong <joshua.yeong@starfivetech.com>

firmware: riscv: Add RPMI performance service

This commit adds a mailbox client based core for the RISC-V RPMI performance
service group. It manages the mailbox channel, enumerates performance
domains, and exports interfaces for front-end drivers.

> diff --git a/drivers/firmware/riscv/riscv-rpmi-performance.c b/drivers/firmware/riscv/riscv-rpmi-performance.c
> index 0000000000000..d44802f07ab0a 100644
> --- /dev/null
> +++ b/drivers/firmware/riscv/riscv-rpmi-performance.c
[ ... ]
> +static int rpmi_perf_extract_attributes(struct rpmi_perf_get_fast_channel_attributes_rx *rx,
> +					struct rpmi_perf_domain *domain,
> +					struct rpmi_fc_info *fc, u32 service_id)
> +{
> +	u64 region_size = domain->perf->fast_channel.size;
> +	u64 phys_offset, phys_addr, db_phys_addr;
[ ... ]
> +	phys_addr = domain->perf->fast_channel.addr + phys_offset;
> +
> +	addr = devm_ioremap(domain->dev, phys_addr, size);

[Severity: High]
Does this code silently truncate the 64-bit physical address on 32-bit platforms?

The variables phys_addr and db_phys_addr are 64-bit u64 values. However,
devm_ioremap() takes a resource_size_t as its second argument. On 32-bit
RISC-V platforms without CONFIG_PHYS_ADDR_T_64BIT, resource_size_t is 32-bit.
If the firmware supplies an address above 4GB, the upper 32 bits are silently
truncated, mapping the wrong physical memory. This could lead to memory
corruption on subsequent fast channel or doorbell iowrite32 operations.

Should the driver explicitly validate that phys_addr and db_phys_addr do not
exceed (resource_size_t)~0ULL?

> +	if (!addr) {
> +		dev_err(domain->dev,
> +			"failed to get fastchannel virtual addr in domain: %d\n",
> +			domain->id);
> +		return -EADDRNOTAVAIL;
> +	}
> +
> +	switch (service_id) {
> +	case RPMI_PERF_SRV_GET_LEVEL:
> +		fc->get_addr = addr;
> +		break;
> +	case RPMI_PERF_SRV_SET_LEVEL:
> +		if (SUPPORTS_DOORBELL(flags)) {
> +			db = devm_kzalloc(domain->dev,
> +					  sizeof(struct rpmi_fc_db_info),
> +					  GFP_KERNEL);
> +			if (!db) {
> +				ret = -ENOMEM;
> +				goto err_unmap;
> +			}
> +
> +			db_addr_width = 1 << (DOORBELL_REG_WIDTH(flags) + 3);
> +			db_phys_addr = le32_to_cpu(rx->db_addr_low);
> +			db_phys_addr |= (u64)le32_to_cpu(rx->db_addr_high) << 32;
> +
> +			/* The doorbell can be any register; map only its width. */
> +			db_addr = devm_ioremap(domain->dev, db_phys_addr,
> +					       db_addr_width / BITS_PER_BYTE);

[Severity: High]
Does this face the same 64-bit to 32-bit silent truncation issue as phys_addr
above when passed to devm_ioremap()?

> +			if (!db_addr) {
> +				devm_kfree(domain->dev, db);
> +				ret = -EADDRNOTAVAIL;
> +				goto err_unmap;
> +			}
[ ... ]
> +static struct platform_driver rpmi_perf_platdrv = {
> +	.driver = {
> +		.name = "riscv-rpmi-performance",
> +		.of_match_table = rpmi_perf_of_match,
> +	},
> +	.probe = rpmi_perf_probe,
> +};

[Severity: High]
Does this driver allow manual sysfs unbinding which could lead to a
use-after-free?

If a privileged user unbinds the riscv-rpmi-performance driver (or its child)
via sysfs while consumers are still attached, pm_genpd_remove() returns -EBUSY
and leaves the domain registered in the PM core. Because the error is ignored,
devres proceeds to free the provider and genpd structs, as well as the
rpmi_perf_domain structures, and unmaps the fast channels. When a consumer
later transitions its performance state, it dereferences the freed memory and
crashes the kernel.

Should the platform driver set .suppress_bind_attrs = true to prevent manual
sysfs unbinds?

> +
> +module_platform_driver(rpmi_perf_platdrv);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008091032.2832333-1-joshua.yeong@starfivetech.com?part=4