Re: [PATCH 1/2] vfio/cdx: Fix NULL pointer dereference in interrupt trigger path

[Alex Williamson <alex@shazbot.org>]

On Thu, 2 Apr 2026 10:14:24 +0530
Prasanna Kumar T S M <ptsm@linux.microsoft.com> wrote:

> On 01-04-2026 23:52, Alex Williamson wrote:
> > On Wed, 1 Apr 2026 15:11:17 +0530
> > "Gupta, Nipun" <nipun.gupta@amd.com> wrote:
> >   
> >> On 20-03-2026 15:49, Prasanna Kumar T S M wrote:  
> >>> Add validation to ensure MSI is configured before accessing cdx_irqs
> >>> array in vfio_cdx_set_msi_trigger(). Without this check, userspace
> >>> can trigger a NULL pointer dereference by calling VFIO_DEVICE_SET_IRQS
> >>> with VFIO_IRQ_SET_DATA_BOOL or VFIO_IRQ_SET_DATA_NONE flags before
> >>> ever setting up interrupts via VFIO_IRQ_SET_DATA_EVENTFD.
> >>>
> >>> The vfio_cdx_msi_enable() function allocates the cdx_irqs array and
> >>> sets config_msi to 1 only when called through the EVENTFD path. The
> >>> trigger loop (for DATA_BOOL/DATA_NONE) assumed this had already been
> >>> done, but there was no enforcement of this call ordering.
> >>>
> >>> This matches the protection used in the PCI VFIO driver where
> >>> vfio_pci_set_msi_trigger() checks irq_is() before the trigger loop.
> >>>
> >>> Fixes: 848e447e000c ("vfio/cdx: add interrupt support")
> >>> Cc: stable@vger.kernel.org
> >>> Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>  
> >>
> >> Acked-by: Nipun Gupta <nipun.gupta@amd.com>  
> > 
> > It's an improvement, but I think it also highlights that interrupt
> > setup for vfio-cdx devices is racy.  I think it should adopt a mutex on
> > the vfio_cdx_device that is acquired with a guard in
> > vfio_cdx_set_irqs_ioctl().  That would make config_msi stable for this
> > test.  Thanks,
> > 
> > Alex  
> 
> This patch is fixing a specific problem. User space can make VFIO_* 
> calls in a specific order to trigger NULL pointer access. This will not 
> get fixed with a mutex.

I'm not saying the fix is wrong, I'm saying it's incomplete.  This
fixes the specific case where config_msi is not set prior to the ioctl,
but it doesn't consider concurrency where config_msi may be set when
tested, but race with a call to vfio_cdx_msi_disable().  I think the fix
needs both the validation of config_msi and serialization via a mutex
such that the test remains valid through the whole ioctl path.  Thanks,

Alex