[PATCH] vfio/pci: Replace abort() with g_assert_not_reached()

Cédric Le Goater posted 1 patch 1 week, 6 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260506152353.1657838-1-clg@redhat.com
Maintainers: Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>
hw/vfio/pci.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
[PATCH] vfio/pci: Replace abort() with g_assert_not_reached()
Posted by Cédric Le Goater 1 week, 6 days ago
This check was originally introduced in commit b3ebc10c373e
("vfio-pci: Add debug config options to disable MSI/X KVM support") as
part of a debug block to retrieve the MSI/MSIX message, and was later
moved by commit 0de70dc7bab1 ("vfio/pci: Rename MSI/X functions for
easier tracing") into the main interrupt handling path, becoming
production code.

Under normal conditions, this code path cannot be reached because the
BQL serializes all handler registration, vdev->interrupt updates, and
handler removal. Replace abort() with g_assert_not_reached(), which is
preferred nowdays, and add a comment clarifying the purpose.

Cc: Alex Williamson <alex@shazbot.org>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/vfio/pci.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 09acd3f6082b0a9c5223780bf7e9847b57424cf7..7ca2ff4e7c1cb570cd60d35454b33b5506dae36e 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -453,7 +453,12 @@ static void vfio_msi_interrupt(void *opaque)
         get_msg = msi_get_message;
         notify = msi_notify;
     } else {
-        abort();
+        /*
+         * Interrupt state transitions (MSI/MSI-X -> NONE/INTx) are
+         * protected by the BQL, and eventfd handlers are strictly
+         * unregistered before vdev->interrupt is modified.
+         */
+        g_assert_not_reached();
     }
 
     msg = get_msg(pdev, nr);
-- 
2.54.0


Re: [PATCH] vfio/pci: Replace abort() with g_assert_not_reached()
Posted by Philippe Mathieu-Daudé 1 week, 5 days ago
On 6/5/26 17:23, Cédric Le Goater wrote:
> This check was originally introduced in commit b3ebc10c373e
> ("vfio-pci: Add debug config options to disable MSI/X KVM support") as
> part of a debug block to retrieve the MSI/MSIX message, and was later
> moved by commit 0de70dc7bab1 ("vfio/pci: Rename MSI/X functions for
> easier tracing") into the main interrupt handling path, becoming
> production code.
> 
> Under normal conditions, this code path cannot be reached because the
> BQL serializes all handler registration, vdev->interrupt updates, and
> handler removal. Replace abort() with g_assert_not_reached(), which is
> preferred nowdays, and add a comment clarifying the purpose.
> 
> Cc: Alex Williamson <alex@shazbot.org>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>   hw/vfio/pci.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 09acd3f6082b0a9c5223780bf7e9847b57424cf7..7ca2ff4e7c1cb570cd60d35454b33b5506dae36e 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -453,7 +453,12 @@ static void vfio_msi_interrupt(void *opaque)
>           get_msg = msi_get_message;
>           notify = msi_notify;
>       } else {
> -        abort();
> +        /*
> +         * Interrupt state transitions (MSI/MSI-X -> NONE/INTx) are
> +         * protected by the BQL, and eventfd handlers are strictly
> +         * unregistered before vdev->interrupt is modified.
> +         */
> +        g_assert_not_reached();
>       }

Could be more readable using switch(vdev->interrupt).

Re: [PATCH] vfio/pci: Replace abort() with g_assert_not_reached()
Posted by Cédric Le Goater 1 week, 2 days ago
On 5/7/26 15:16, Philippe Mathieu-Daudé wrote:
> On 6/5/26 17:23, Cédric Le Goater wrote:
>> This check was originally introduced in commit b3ebc10c373e
>> ("vfio-pci: Add debug config options to disable MSI/X KVM support") as
>> part of a debug block to retrieve the MSI/MSIX message, and was later
>> moved by commit 0de70dc7bab1 ("vfio/pci: Rename MSI/X functions for
>> easier tracing") into the main interrupt handling path, becoming
>> production code.
>>
>> Under normal conditions, this code path cannot be reached because the
>> BQL serializes all handler registration, vdev->interrupt updates, and
>> handler removal. Replace abort() with g_assert_not_reached(), which is
>> preferred nowdays, and add a comment clarifying the purpose.
>>
>> Cc: Alex Williamson <alex@shazbot.org>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>   hw/vfio/pci.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 09acd3f6082b0a9c5223780bf7e9847b57424cf7..7ca2ff4e7c1cb570cd60d35454b33b5506dae36e 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -453,7 +453,12 @@ static void vfio_msi_interrupt(void *opaque)
>>           get_msg = msi_get_message;
>>           notify = msi_notify;
>>       } else {
>> -        abort();
>> +        /*
>> +         * Interrupt state transitions (MSI/MSI-X -> NONE/INTx) are
>> +         * protected by the BQL, and eventfd handlers are strictly
>> +         * unregistered before vdev->interrupt is modified.
>> +         */
>> +        g_assert_not_reached();
>>       }
> 
> Could be more readable using switch(vdev->interrupt).
> 

I agree. But let's have another patch for it if you don't mind.
This change is about the rational for the abort() and it was
reported to the security list.

Thanks,

C.


Re: [PATCH] vfio/pci: Replace abort() with g_assert_not_reached()
Posted by Philippe Mathieu-Daudé 1 week, 1 day ago
On 10/5/26 18:12, Cédric Le Goater wrote:
> On 5/7/26 15:16, Philippe Mathieu-Daudé wrote:
>> On 6/5/26 17:23, Cédric Le Goater wrote:
>>> This check was originally introduced in commit b3ebc10c373e
>>> ("vfio-pci: Add debug config options to disable MSI/X KVM support") as
>>> part of a debug block to retrieve the MSI/MSIX message, and was later
>>> moved by commit 0de70dc7bab1 ("vfio/pci: Rename MSI/X functions for
>>> easier tracing") into the main interrupt handling path, becoming
>>> production code.
>>>
>>> Under normal conditions, this code path cannot be reached because the
>>> BQL serializes all handler registration, vdev->interrupt updates, and
>>> handler removal. Replace abort() with g_assert_not_reached(), which is
>>> preferred nowdays, and add a comment clarifying the purpose.
>>>
>>> Cc: Alex Williamson <alex@shazbot.org>
>>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>>> ---
>>>   hw/vfio/pci.c | 7 ++++++-
>>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>>> index 
>>> 09acd3f6082b0a9c5223780bf7e9847b57424cf7..7ca2ff4e7c1cb570cd60d35454b33b5506dae36e 100644
>>> --- a/hw/vfio/pci.c
>>> +++ b/hw/vfio/pci.c
>>> @@ -453,7 +453,12 @@ static void vfio_msi_interrupt(void *opaque)
>>>           get_msg = msi_get_message;
>>>           notify = msi_notify;
>>>       } else {
>>> -        abort();
>>> +        /*
>>> +         * Interrupt state transitions (MSI/MSI-X -> NONE/INTx) are
>>> +         * protected by the BQL, and eventfd handlers are strictly
>>> +         * unregistered before vdev->interrupt is modified.
>>> +         */
>>> +        g_assert_not_reached();
>>>       }
>>
>> Could be more readable using switch(vdev->interrupt).
>>
> 
> I agree. But let's have another patch for it if you don't mind.
> This change is about the rational for the abort() and it was
> reported to the security list.

Sure, just suggesting, not aiming to block!

Re: [PATCH] vfio/pci: Replace abort() with g_assert_not_reached()
Posted by Alex Williamson 1 week, 5 days ago

On Wed, May 6, 2026, at 9:23 AM, Cédric Le Goater wrote:
> This check was originally introduced in commit b3ebc10c373e
> ("vfio-pci: Add debug config options to disable MSI/X KVM support") as
> part of a debug block to retrieve the MSI/MSIX message, and was later
> moved by commit 0de70dc7bab1 ("vfio/pci: Rename MSI/X functions for
> easier tracing") into the main interrupt handling path, becoming
> production code.
>
> Under normal conditions, this code path cannot be reached because the
> BQL serializes all handler registration, vdev->interrupt updates, and
> handler removal. Replace abort() with g_assert_not_reached(), which is
> preferred nowdays, and add a comment clarifying the purpose.
>
> Cc: Alex Williamson <alex@shazbot.org>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>  hw/vfio/pci.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 
> 09acd3f6082b0a9c5223780bf7e9847b57424cf7..7ca2ff4e7c1cb570cd60d35454b33b5506dae36e 
> 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -453,7 +453,12 @@ static void vfio_msi_interrupt(void *opaque)
>          get_msg = msi_get_message;
>          notify = msi_notify;
>      } else {
> -        abort();
> +        /*
> +         * Interrupt state transitions (MSI/MSI-X -> NONE/INTx) are
> +         * protected by the BQL, and eventfd handlers are strictly
> +         * unregistered before vdev->interrupt is modified.
> +         */
> +        g_assert_not_reached();
>      }
> 
>      msg = get_msg(pdev, nr);
> -- 
> 2.54.0

Acked-by: Alex Williamson <alex@shazbot.org>