Re: [PATCH v20 8/9] firmware: arm_rmm: Add wrappers for Realm related RMI commands

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] arm_rmm: Range-based RMI wrappers drop progress indicator on error, risking GPF
- [High] arm_rmm: SRO cancellation flow aborts prematurely, leaving SRO dangling
--

commit 206f9f5530d9fa5eb129cd6fc6e598e72857683a
Author: Steven Price <steven.price@arm.com>

firmware: arm_rmm: Add wrappers for Realm related RMI commands

This commit introduces static inline wrappers for executing Realm
Management Interface (RMI) commands. These wrappers call into lower-level
SMC execution functions to manage Realm guests.

> diff --git a/include/linux/arm-rmi-cmds.h b/include/linux/arm-rmi-cmds.h
> index 5d005054f3c6c..4f0d6a8006a4a 100644
> --- a/include/linux/arm-rmi-cmds.h
> +++ b/include/linux/arm-rmi-cmds.h
> @@ -104,4 +104,464 @@ static inline bool is_rmi_available(void)
>  }
>  #endif	/* CONFIG_ARM_RMM_RMI */
>  
> +
> +/**
> + * rmi_rtt_data_map_init() - Create a mapping at protected IPA, copying contents
> + *			     from a given non-secure source granule.
> + * @rd: PA of the RD
> + * @data: PA of the target granule mapped in the guest
> + * @ipa: IPA at which the granule @data will be mapped in the guest
> + * @src: PA of the source granule with contents
> + * @flags: RMI_MEASURE_CONTENT if the contents should be measured
> + *
> + * Create a mapping from Protected IPA space to conventional memory, copying
> + * contents from a Non-secure Granule provided by the caller.
> + *
> + * Return: 0 on success, positive RMI result code or negative Linux error code
> + */
> +static inline long rmi_rtt_data_map_init(unsigned long rd, unsigned long data,
> +					 unsigned long ipa, unsigned long src,
> +					 unsigned long flags)
> +{
> +	struct arm_smccc_1_2_regs regs = {
> +		SMC_RMI_RTT_DATA_MAP_INIT, rd, data, ipa, src, flags
> +	};
> +
> +	return rmi_sro_execute(&regs);

[Severity: High]
Does the SRO cancellation flow in the underlying rmi_sro_execute() and
rmi_sro_memxfer_execute() abort prematurely when returning to these wrappers?

In rmi_sro_execute(), the cancellation path does this:

	rmi_op_cancel(sro_handle, regs);
	cancelled = true;

According to the RMM specification, SMC_RMI_OP_CANCEL returns RMI_SUCCESS when
accepted. Since RMI_SUCCESS is 0, the loop condition in rmi_sro_execute():

	while (RMI_RESULT_STATUS(regs->a0) == RMI_INCOMPLETE)

evaluates to false. This breaks out of the loop and skips the mandatory
RMI_OP_CONTINUE calls needed to fully unwind the SRO.

Similarly, in rmi_sro_memxfer_execute(), there is an explicit assertion:

	if (WARN_ON_ONCE(RMI_RESULT_STATUS(regs->a0) != RMI_INCOMPLETE))
		return ret;

which immediately aborts when rmi_op_cancel() correctly returns RMI_SUCCESS.

Can this leave SRO operations permanently in the CANCELLING state within the
RMM, leading to resource leaks and preventing realms from being properly
destroyed?

[ ... ]

> +static inline long rmi_rtt_data_unmap(unsigned long rd,
> +				      unsigned long base,
> +				      unsigned long top,
> +				      unsigned long flags,
> +				      unsigned long oaddr,
> +				      unsigned long *out_top,
> +				      unsigned long *out_range,
> +				      unsigned long *out_count)
> +{
> +	struct arm_smccc_1_2_regs regs = {
> +		SMC_RMI_RTT_DATA_UNMAP, rd, base, top, flags, oaddr
> +	};
> +	long ret;
> +
> +	ret = rmi_sro_execute(&regs);
> +
> +	if (ret != RMI_SUCCESS)
> +		return ret;
> +
> +	if (out_top)
> +		*out_top = regs.a1;

[Severity: Critical]
If a range-based RMI SRO executes partially and returns an error (such as
RMI_ERROR_INPUT), the architecture specifies that the address of the first
unprocessed granule is returned in regs.a1.

Does this wrapper drop the Top progress marker on failure?

By only populating out_top when ret == RMI_SUCCESS (or by returning early on
failure as seen here), callers might evaluate their progress as zero on
partial failure.

In paths like rmi_delegate_range(), this unpopulated out_top causes out_phys
to evaluate to the start address, which bypasses rmi_undelegate_range().
Could this lead to leaking partially processed granules to the allocator,
causing Granule Protection Faults (GPF) when the host accesses them later?

The same pattern appears in rmi_rtt_data_map(), rmi_rtt_unprot_map(),
rmi_rtt_unprot_unmap(), and rmi_granule_range_delegate().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929221623.1342076-1-suzuki.poulose@arm.com?part=8