(re-sending, as I still don't see the mail having appeared on the list)
On 23.12.2019 15:04, Alexandru Stefan ISAILA wrote:
> Changes since V5:
> - Add black lines
Luckily no color comes through in plain text mails ;-)
> --- a/xen/arch/x86/mm/mem_access.c
> +++ b/xen/arch/x86/mm/mem_access.c
> @@ -366,11 +366,12 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn,
> uint32_t nr,
> #ifdef CONFIG_HVM
> if ( altp2m_idx )
> {
> - if ( altp2m_idx >= MAX_ALTP2M ||
> - d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) )
> + if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
Stray blank after >= .
> + d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
I accept you can't (currently) use array_access_nospec() here,
but ...
> + mfn_x(INVALID_MFN) )
> return -EINVAL;
>
> - ap2m = d->arch.altp2m_p2m[altp2m_idx];
> + ap2m = d->arch.altp2m_p2m[array_index_nospec(altp2m_idx,
> MAX_ALTP2M)];
... I don't see why you still effectively open-code it here.
> @@ -425,11 +426,12 @@ long p2m_set_mem_access_multi(struct domain *d,
> #ifdef CONFIG_HVM
> if ( altp2m_idx )
> {
> - if ( altp2m_idx >= MAX_ALTP2M ||
> - d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) )
> + if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
> + d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
> + mfn_x(INVALID_MFN) )
> return -EINVAL;
>
> - ap2m = d->arch.altp2m_p2m[altp2m_idx];
> + ap2m = d->arch.altp2m_p2m[array_index_nospec(altp2m_idx,
> MAX_ALTP2M)];
Same two remarks here then, and again further down.
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -2577,6 +2577,8 @@ int p2m_init_altp2m_by_id(struct domain *d, unsigned
> int idx)
> if ( idx >= MAX_ALTP2M )
> return rc;
>
> + idx = array_index_nospec(idx, MAX_ALTP2M);
> +
> altp2m_list_lock(d);
>
> if ( d->arch.altp2m_eptp[idx] == mfn_x(INVALID_MFN) )
What about this array access?
> @@ -2618,6 +2620,8 @@ int p2m_destroy_altp2m_by_id(struct domain *d, unsigned
> int idx)
> if ( !idx || idx >= MAX_ALTP2M )
> return rc;
>
> + idx = array_index_nospec(idx, MAX_ALTP2M);
There's a d->arch.altp2m_eptp[] access down from here too. I'm not
going to look further. Please get things into consistent shape while
you do this transformation.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
