Re: [PATCH] x86/vMsi-x: check whether the msixtbl_list has been initialized or not when accessing it

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 29/07/16 02:35, Chao Gao wrote:
> MSI-x tables' initialization had been detered in the commit
> 74c6dc2d0ac4dcab0c6243cdf6ed550c1532b798. If an assigned device does not support
> MSI-x, the msixtbl_list won't be initialized. Howerver, both of following paths
>     XEN_DOMCTL_bind_pt_irq
>         pt_irq_create_bind
>             msixtbl_pt_register
> and
>     XEN_DOMCTL_unbind_pt_irq
>         pt_irq_destroy_bind
>             msixtbl_pt_unregister
> do not check this case and will cause Xen panic consequently.
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>

This issue was already reported and I provided a fix in

https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=db0eee0a071e2e3e18e79d21a9b1d6724edeeeb3

However, looking at your patch, I forgot to fix the
msixtbl_pt_register() path, so your patch is still necessary.

Please rebase this patch onto the staging branch which has the
aformentioned fix in, at which point it can be accepted.  Just one note.

> ---
>  xen/arch/x86/hvm/vmsi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
> index e418b98..e0d710b 100644
> --- a/xen/arch/x86/hvm/vmsi.c
> +++ b/xen/arch/x86/hvm/vmsi.c
> @@ -449,7 +449,7 @@ int msixtbl_pt_register(struct domain *d, struct pirq *pirq, uint64_t gtable)
>      ASSERT(pcidevs_locked());
>      ASSERT(spin_is_locked(&d->event_lock));
>  
> -    if ( !has_vlapic(d) )
> +    if ( !has_vlapic(d) || !d->arch.hvm_domain.msixtbl_list.next )

You can drop the vlapic() check, as it is redundant with whether msixtbl
is enabled or not.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel