Kind regards,
Henry
> -----Original Message-----
> From: Juergen Gross <jgr...@suse.com>
> Sent: Friday, September 9, 2022 5:26 PM
> To: Jan Beulich <jbeul...@suse.com>
> Cc: Henry Wang <henry.w...@arm.com>; Andrew Cooper
> <andrew.coop...@citrix.com>; George Dunlap <george.dun...@citrix.com>;
> Julien Grall <jul...@xen.org>; Stefano Stabellini <sstabell...@kernel.org>;
> Wei Liu <w...@xen.org>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH] xen/gnttab: fix gnttab_acquire_resource()
>
> On 09.09.22 11:15, Jan Beulich wrote:
> > 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 <jgr...@suse.com>
> >> ---
> >> 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.
>
> Okay.
>
> >
> >> @@ -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"?
>
> I can do that, if you like that better.
>
>
> Juergen
