vfio/iommufd: Fix SIGSEV in iommufd_cdev_attach()

[PATCH] vfio/iommufd: Fix SIGSEV in iommufd_cdev_attach()

Posted by Zhenzhong Duan 1 year ago

When iommufd_cdev_ram_block_discard_disable() fails for whatever reason,
errp should be set or else SIGSEV is triggered in vfio_realize() when
error_prepend() is called.

By this chance, use the same error message for both legacy and iommufd
backend.

Fixes: 5ee3dc7af785 ("vfio/iommufd: Implement the iommufd backend")
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/iommufd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 3490a8f1eb..df61edffc0 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -515,8 +515,8 @@ static bool iommufd_cdev_attach(const char *name, VFIODevice *vbasedev,
         } else {
             ret = iommufd_cdev_ram_block_discard_disable(true);
             if (ret) {
-                error_setg(errp,
-                              "Cannot set discarding of RAM broken (%d)", ret);
+                error_setg_errno(errp, -ret,
+                                 "Cannot set discarding of RAM broken");
                 goto err_discard_disable;
             }
             goto found_container;
@@ -544,6 +544,7 @@ static bool iommufd_cdev_attach(const char *name, VFIODevice *vbasedev,
 
     ret = iommufd_cdev_ram_block_discard_disable(true);
     if (ret) {
+        error_setg_errno(errp, -ret, "Cannot set discarding of RAM broken");
         goto err_discard_disable;
     }
 
-- 
2.34.1

Re: [PATCH] vfio/iommufd: Fix SIGSEV in iommufd_cdev_attach()

Posted by Cédric Le Goater 1 year ago

On 1/16/25 11:23, Zhenzhong Duan wrote:
> When iommufd_cdev_ram_block_discard_disable() fails for whatever reason,
> errp should be set or else SIGSEV is triggered in vfio_realize() when
> error_prepend() is called.
> 
> By this chance, use the same error message for both legacy and iommufd
> backend.
> 
> Fixes: 5ee3dc7af785 ("vfio/iommufd: Implement the iommufd backend")
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>


Applied to vfio-next.

Thanks,

C.


> ---
>   hw/vfio/iommufd.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 3490a8f1eb..df61edffc0 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -515,8 +515,8 @@ static bool iommufd_cdev_attach(const char *name, VFIODevice *vbasedev,
>           } else {
>               ret = iommufd_cdev_ram_block_discard_disable(true);
>               if (ret) {
> -                error_setg(errp,
> -                              "Cannot set discarding of RAM broken (%d)", ret);
> +                error_setg_errno(errp, -ret,
> +                                 "Cannot set discarding of RAM broken");
>                   goto err_discard_disable;
>               }
>               goto found_container;
> @@ -544,6 +544,7 @@ static bool iommufd_cdev_attach(const char *name, VFIODevice *vbasedev,
>   
>       ret = iommufd_cdev_ram_block_discard_disable(true);
>       if (ret) {
> +        error_setg_errno(errp, -ret, "Cannot set discarding of RAM broken");
>           goto err_discard_disable;
>       }
>

Re: [PATCH] vfio/iommufd: Fix SIGSEV in iommufd_cdev_attach()

Posted by Eric Auger 1 year ago

Hi Zhenzhong,


On 1/16/25 11:23 AM, Zhenzhong Duan wrote:
> When iommufd_cdev_ram_block_discard_disable() fails for whatever reason,
> errp should be set or else SIGSEV is triggered in vfio_realize() when
> error_prepend() is called.
>
> By this chance, use the same error message for both legacy and iommufd
> backend.
>
> Fixes: 5ee3dc7af785 ("vfio/iommufd: Implement the iommufd backend")
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  hw/vfio/iommufd.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 3490a8f1eb..df61edffc0 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -515,8 +515,8 @@ static bool iommufd_cdev_attach(const char *name, VFIODevice *vbasedev,
>          } else {
>              ret = iommufd_cdev_ram_block_discard_disable(true);
>              if (ret) {
> -                error_setg(errp,
> -                              "Cannot set discarding of RAM broken (%d)", ret);
> +                error_setg_errno(errp, -ret,
> +                                 "Cannot set discarding of RAM broken");
>                  goto err_discard_disable;
>              }
>              goto found_container;
> @@ -544,6 +544,7 @@ static bool iommufd_cdev_attach(const char *name, VFIODevice *vbasedev,
>  
>      ret = iommufd_cdev_ram_block_discard_disable(true);
>      if (ret) {
> +        error_setg_errno(errp, -ret, "Cannot set discarding of RAM broken");
>          goto err_discard_disable;
>      }
>  
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks!

Eric