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

[Prasanna Kumar T S M <ptsm@linux.microsoft.com>]

On 02-04-2026 19:46, Alex Williamson wrote:
> 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

Thanks for the suggestion. Yes, the concurrency issue exists. It exists 
even before this patch and it has to be fixed.

The CDX folks will be able to do this faster than me as they have deep 
expertise on this.

Thanks,
Prasanna Kumar