Re: [PATCH] xen/gnttab: fix gnttab_acquire_resource()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.09.2022 10:09, Juergen Gross wrote:
> Commit 9dc46386d89d ("gnttab: work around "may be used uninitialized"
> warning") was wrong, as vaddr can legitimately be NULL in case
> XENMEM_resource_grant_table_id_status was specified for a grant table
> v1. This would result in crashes in debug builds due to
> ASSERT_UNREACHABLE() triggering.
> 
> Basically revert said commit, but keep returning -ENODATA in that case.
> 
> Fixes: 9dc46386d89d ("gnttab: work around "may be used uninitialized" 
> warning")
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
> Might be considered for 4.17 and for backporting
> ---
>  xen/common/grant_table.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index ad773a6996..68e7f1df38 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -4125,7 +4125,10 @@ int gnttab_acquire_resource(
>  
>      case XENMEM_resource_grant_table_id_status:
>          if ( gt->gt_version != 2 )
> +        {
> +            rc = -ENODATA;
>              break;
> +        }

This path is supposed to produce -EINVAL.

> @@ -4135,17 +4138,6 @@ int gnttab_acquire_resource(
>          break;
>      }
>  
> -    /*
> -     * Some older toolchains can't spot that vaddrs won't remain 
> uninitialized
> -     * on non-error paths, and hence it needs setting to NULL at the top of 
> the
> -     * function.  Leave some runtime safety.
> -     */
> -    if ( !vaddrs )

I guess this wants amending by "&& !rc"?

Jan