Hi Bertrand, On Mon, Mar 2, 2026 at 10:58 AM Bertrand Marquis <[email protected]> wrote: > > Hi Jens, > > > On 2 Mar 2026, at 10:32, Jens Wiklander <[email protected]> wrote: > > > > Hi Bertrand, > > > > On Mon, Mar 2, 2026 at 9:51 AM Bertrand Marquis > > <[email protected]> wrote: > >> > >> Hi Jens, > >> > >>> On 27 Feb 2026, at 11:39, Jens Wiklander <[email protected]> > >>> wrote: > >>> > >>> Hi Bertrand, > >>> > >>> On Wed, Feb 25, 2026 at 11:02 AM Bertrand Marquis > >>> <[email protected]> wrote: > >>>> > >>>> FFA_PARTITION_INFO_GET currently queries the SPMC on each call and walks > >>>> the > >>>> RX buffer every time. The SP list is expected to be static, so repeated > >>>> firmware calls and validation are unnecessary. > >>>> > >>>> Cache the SPMC partition-info list at init time, keeping only secure > >>>> endpoints, and reuse the cached entries for SP count and partition-info > >>>> responses. Initialize the VM create/destroy subscriber lists from the > >>>> cached > >>>> list and free the cache on init failure. > >>>> > >>>> SP partition info now reflects the init-time snapshot and will not > >>>> change. > >>>> > >>>> Signed-off-by: Bertrand Marquis <[email protected]> > >>>> --- > >>>> xen/arch/arm/tee/ffa_partinfo.c | 205 +++++++++++++++++++++----------- > >>>> 1 file changed, 138 insertions(+), 67 deletions(-) > >>>> [snip] > >>>> +static bool ffa_sp_list_cache_init(const void *buf, uint32_t count, > >>>> + uint32_t fpi_size) > >>>> +{ > >>>> + const uint8_t *src = buf; > >>>> + uint32_t secure_count = 0; > >>>> + uint32_t n, idx = 0; > >>>> + bool warned = false; > >>>> + > >>>> + if ( fpi_size < sizeof(struct ffa_partition_info_1_0) || > >>>> + fpi_size >= FFA_PAGE_SIZE ) > >>>> + return false; > >>> > >>> Would it make sense to check that fpi_size is well aligned with struct > >>> ffa_partition_info_1_0? If it's an odd size, we'll make unaligned > >>> accesses below with memcpy(). But perhaps that's a bit much. The SPMC > >>> isn't supposed to provide garbage. > >> > >> Memcpy should prevent issues even if accesses are not aligned. > >> If we had this test, we cannot return an error to the SPMC so we would > >> have to > >> generate one to the caller. It is simpler i think to handle non-aligned as > >> we do not > >> expect the SPMC to generate such a case. > >> Tell me if you agree. > > > > We dereference fpi below, and depending on compiler flags and pointer > > types, memcpy() might not be safe with unaligned pointers. > > From 6.3.2.3 Pointers, paragraph 7, in the C standard: > > "A pointer to an object type may be converted to a pointer to a > > different object type. If the > > resulting pointer is not correctly aligned for the referenced type, > > the behavior is > > undefined." > > > > I've seen past examples where the compiler optimized memcpy() in a way > > that breaks with unaligned pointers. > > > > We don't expect the test above to fail, but if it does we will not use > > the secure firmware. I think refusing unexpected sizes is even > > simpler. It should make finding eventual errors much easier. > > In the ffa spec, the size can grow and this is why there is a size field. > FF-A expect that we either ignore or copy without looking the extra > content. > I think we should not be dependent on any alignment so we need to > make sure we do the copy in a way that is robust to non alignments. > > > > > So my question above is whether it's worth checking that fpi_size is > > well-aligned, or if it's so unlikely that we don't need to consider > > it. > > I think we need to find a solution where we handle properly things even > if the size is not aligned.
I agree, that's the most robust. > I though using memcpy would protect against > that but maybe we need to use a stronger solution to ensure that works > even if data is unaligned. memcpy() can be used, but the pointer values must not pass through a struct ffa_partition_info_1_0 pointer or such. Cheers, Jens
