Re: [PATCH] hw/pci: Make msix_init take a uint32_t for nentries

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 7/11/25 14:10, Peter Maydell wrote:
> msix_init() and msix_init_exclusive_bar() take an "unsigned short"
> argument for the number of MSI-X vectors to try to use.  This is big
> enough for the maximum permitted number of vectors, which is 2048.
> Unfortunately, we have several devices (most notably virtio) which
> allow the user to specify the desired number of vectors, and which
> use uint32_t properties for this.  If the user sets the property to a
> value that is too big for a uint16_t, the value will be truncated
> when it is passed to msix_init(), and msix_init() may then return
> success if the truncated value is a valid one.
> 
> The resulting mismatch between the number of vectors the msix code
> thinks the device has and the number of vectors the device itself
> thinks it has can cause assertions, such as the one in issue 2631,
> where "-device virtio-mouse-pci,vectors=19923041" is interpreted by
> msix as "97 vectors" and by the virtio-pci layer as "19923041
> vectors"; a guest attempt to access vector 97 thus passes the
> virtio-pci bounds checking and hits an essertion in
> msix_vector_use().
> 
> Avoid this by making msix_init() and its wrapper function
> msix_init_exclusive_bar() take the number of vectors as a uint32_t.
> The erroneous command line will now produce the warning
> 
>   qemu-system-i386: -device virtio-mouse-pci,vectors=19923041:
>     warning: unable to init msix vectors to 19923041
> 
> and proceed without crashing.  (The virtio device warns and falls
> back to not using MSIX, rather than complaining that the option is
> not a valid value this is the same as the existing behaviour for
> values that are beyond the MSI-X maximum possible value but fit into
> a 16-bit integer, like 2049.)
> 
> To ensure this doesn't result in potential overflows in calculation
> of the BAR size in msix_init_exclusive_bar(), we duplicate the
> nentries error-check from msix_init() at the top of
> msix_init_exclusive_bar(), so we know nentries is sane before we
> start using it.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2631
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Technically this fixes an assertion, but only if the command
> line is daft, so I didn't think it worth backporting to stable.
> ---
>   include/hw/pci/msix.h |  4 ++--
>   hw/pci/msix.c         | 10 ++++++++--
>   2 files changed, 10 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>