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

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 09/09/2022 11:24, Juergen Gross wrote:
> Commit 9dc46386d89d ("gnttab: work around "may be used uninitialized"
> warning") was wrong, as vaddrs 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.
>
> Check vaddrs only to be NULL in the rc == 0 case.
>
> Fixes: 9dc46386d89d ("gnttab: work around "may be used uninitialized" 
> warning")
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> Release-acked-by: Henry Wang <Henry.Wang@xxxxxxx>
> ---
> Might be considered for backporting

Definitely for backport.

Given that we do actually have a unit(ish) test capable of triggering
this, I would request that tools/tests/resource/test-resource.c gets
extended in the same commit, so we keep our testing in sync with our
bugfixes.

I've just got all the tests working in XenRT, and my comment about
OSSTest fixes for 4.17 on the community call was first and foremost
about wiring up these tests too.  They're trivial run (even before the
work to give them a consistent interface - probably 4.18 now), so
there's no excuse for them not to run.

In this case, we create the VMs strictly with gnttab v1, so the size()
and map() calls should fail with -ENODEV (or possibly 0 for size). and
not trigger any assertions in Xen.

~Andrew