Re: [XEN PATCH] xen/vpci: Fix UB in mask_write

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Nov 06, 2024 at 02:32:13PM +0000, Mykyta Poturai wrote:
> On 06.11.24 14:42, Roger Pau Monné wrote:
> > On Wed, Nov 06, 2024 at 12:26:55PM +0000, Mykyta Poturai wrote:
> >> On 06.11.2024 10:07, Roger Pau Monné wrote:
> >>> Wait - how can msi->vectors ever be 0?  AFAICT there's no way in the
> >>> MSI logic to configure 0 vectors, there will always be at least 1 vector
> >>> enabled.
> >>>
> >>> Maybe what you want, if this fix is for compliance reasons, is an
> >>> assert unreachable that msi->vectors > 0?
> >>
> >> I did some investigation and figured out that the value of 0 is being
> >> set by guest writing to msi_control_reg. As far as I understand, the
> >> control_write() function only checks that vectors are not greater than
> >> the maximum allowed value, but does not check for 0.
> > 
> > control_write() will set vectors to (1UL << val), so even if user
> > provides val == 0, vectors will be 1.
> > 
> > Can you provide an example input value of control_write() that will
> > lead to msi->vectors == 0?
> > 
> > Is maybe msi_maxvec not set correctly in your use case if you indeed
> > see vectors == 0?
> > 
> > Thanks, Roger.
> 
> Indeed, I have checked and msi_maxvec is set to 0. Thanks for pointing
> this out. I will investigate further why this is happening. It is quite
> strange that it somehow worked on 4.18 with the same problem.

Check whether pdev_msi_init() is called during device addition, as
that's what initializes msi_maxvec.  Another cause could be memory
corruption.

> I will change the check to an assert then, so if something similar
> happens again it can be caught earlier.

Let's try to figure out what causes msi_maxvec to be 0 in your case
and then we can see how to better detect this.  If msi_maxvec needs to
be checked it should likely be done in init_msi().

Regards, Roger.