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

[Juergen Gross <jgross@xxxxxxxx>]

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 <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.
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

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature