Re: [PATCH v2 1/3] arm64: rsi: Add helpers for Arm CCA measurement register operations

[Yeoreum Yun]

> On 09/29 17:57, Yeoreum Yun wrote:
> > From: Sami Mujawar <sami.mujawar@xxxxxxx>
> > 
> > Add static inline helper functions to support reading the Realm
> > Initial Measurement (RIM) and reading/extending the Realm
> > Extensible Measurement (REM) registers.
> > 
> > The indices of the Arm CCA measurement registers, as defined by
> > the Realm Management Monitor specification, are as follows:
> >     Index    Register
> >     0        RIM
> >     1 - 4    REM[0 - 3]
> > 
> > The rsi_measurement_extend() function allows extending REM[0–3]
> > registers with a caller-provided digest (up to 64 bytes).
> > Index 0 (RIM) is read-only and cannot be extended.
> > 
> > The rsi_measurement_read() function allows reading measurement
> > values from RIM (index 0) or REM[0–3] (indices 1–4). The returned
> > digest is expected to be 64 bytes.
> > 
> > Signed-off-by: Sami Mujawar <sami.mujawar@xxxxxxx>
> > ---
> >  include/linux/arm-rsi-cmds.h | 105 ++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 104 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/arm-rsi-cmds.h b/include/linux/arm-rsi-cmds.h
> > index 3f7a6a833993..608343266d81 100644
> > --- a/include/linux/arm-rsi-cmds.h
> > +++ b/include/linux/arm-rsi-cmds.h
> > @@ -1,6 +1,6 @@
> >  /* SPDX-License-Identifier: GPL-2.0-only */
> >  /*
> > - * Copyright (C) 2023 ARM Ltd.
> > + * Copyright (C) 2023 - 2025 ARM Ltd.
> >   */
> >  
> >  #ifndef __LINUX_ARM_RSI_CMDS_H_
> > @@ -36,6 +36,26 @@ static inline bool is_realm_world(void) { return false; }
> >  #define RSI_GRANULE_SHIFT		12
> >  #define RSI_GRANULE_SIZE		(_AC(1, UL) << RSI_GRANULE_SHIFT)
> >  
> > +/*
> > + * Maximum measurement data size in bytes.
> > + * According to the RMM Specification, the width of the RmmRealmMeasurement type
> > + * is 512 bits.
> > + */
> > +#define RSI_MAX_MEASUREMENT_DATA_SIZE_BYTES  64
> > +
> > +/*
> > + * Indices for the Realm Initial Measurement register (RIM) and the Realm
> > + * Extensible Measurement registers (REMs).
> > + * According to the RMM Specification, Realm attributes of a Realm include
> > + * an array of measurement values. The first entry in this array is a RIM.
> > + * The remaining entries in this array are REMs.
> > + */
> > +#define RSI_INDEX_RIM		0
> > +#define RSI_INDEX_REM0		1
> > +#define RSI_INDEX_REM1		2
> > +#define RSI_INDEX_REM2		3
> > +#define RSI_INDEX_REM3		4
> > +
> >  enum ripas {
> >  	RSI_RIPAS_EMPTY = 0,
> >  	RSI_RIPAS_RAM = 1,
> > @@ -236,4 +256,87 @@ static inline unsigned long rsi_attestation_token_continue(phys_addr_t granule,
> >  	return res.a0;
> >  }
> >  
> > +/**
> > + * rsi_measurement_extend - Extend the measurement value to the Realm Extensible
> > + * Measurement (REM).
> > + *
> > + * @idx:		Index of the REM register.
> > + *				Where:
> > + *				Index	Register
> > + *				1 - 4	REM[0-3]
> > + * @digest:		The digest data to be extended.
> > + * @digest_size:	Size of the digest data in bytes.
> > + *
> > + * Returns:
> > + *  On success, returns RSI_SUCCESS.
> > + *  Otherwise, -EINVAL
> > + */
> > +static inline unsigned long rsi_measurement_extend(u32 idx,
> > +						   const u8 *digest,
> > +						   unsigned long digest_size)
> > +{
> > +	struct arm_smccc_1_2_regs regs = { 0 };
> > +
> > +	/*
> > +	 * Index 0 is for RIM (which is Read Only), while
> > +	 * REM[0-3] are indexed from 1 - 4.
> > +	 * The digest size can be at the most 64 bytes.
> > +	 */
> > +	if (!digest || idx < RSI_INDEX_REM0 || idx > RSI_INDEX_REM3 ||
> > +	    digest_size == 0 || digest_size > RSI_MAX_MEASUREMENT_DATA_SIZE_BYTES)
> > +		return -EINVAL;
> > +
> > +	regs.a0 = SMC_RSI_MEASUREMENT_EXTEND;
> > +	regs.a1 = idx;
> > +	regs.a2 = digest_size;
> > +	memcpy(&regs.a3, digest, digest_size);
> 
> With CONFIG_FORTIFY_SOURCE=y, FORTIFY reports the following warning for
> this memcpy:
> 
>   [  899.918673] ------------[ cut here ]------------
>   [  899.918806] memcpy: detected field-spanning write (size 32) of single field "&regs.a3" at ./include/linux/arm-rsi-cmds.h:292 (size 8)
>   [  899.919277] WARNING: ./include/linux/arm-rsi-cmds.h:292 at rsi_measurement_extend+0x104/0x118, CPU#0: dd/123
>   [  900.672180] Modules linked in:
>   [  900.769378] CPU: 0 UID: 0 PID: 123 Comm: dd Not tainted 7.3.0-rc4+ #6 PREEMPT(full)
>   [  901.034515] Hardware name: linux,dummy-virt (DT)
>   [  901.194939] pstate: 61402005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
>   [  901.390491] pc : rsi_measurement_extend+0x104/0x118
>   [  901.530657] lr : rsi_measurement_extend+0x104/0x118
>   [...]
>   [  903.770326] Call trace:
>   [  903.893102]  rsi_measurement_extend+0x104/0x118 (P)
>   [  904.056234]  arm_cca_mr_extend+0x40/0x58
>   [  904.270991]  tm_digest_write+0x90/0x1d8
>   [  904.439429]  sysfs_kf_bin_write+0x98/0xc8
>   [  904.596599]  kernfs_fop_write_iter+0x150/0x1e8
>   [  904.779881]  vfs_write+0x29c/0x450
>   [...]
> 
> Would it make sense to use a union to overlay the struct
> arm_smccc_1_2_regs with an RSI-specific argument layout and copy the
> digest into an explicit 64-byte array, as in commit 221049874b6a
> ("arm64: RSI: fix field-spanning write warning in attestation token
> init")?

Thanks for reporting and suggestion. I'll fix at next-spin.


-- 
Sincerely,
Yeoreum Yun