On Tue, Nov 25, 2025 at 10:50:22PM +0100, Philippe Mathieu-Daudé wrote: > 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 <[email protected]> > > --- > > 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(-) > > Queued, thanks.
I felt it's not 10.2 material, so deferred review. But if you are sure we need it, ok. Acked-by: Michael S. Tsirkin <[email protected]> -- MST
