Re: [PATCH 1/1] hvm.c: Prevent gcc uninitialised var warning

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 23/03/15 15:01, Don Slutz wrote:
> gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3 reports:
> ----------------------------------------------------------------------
> hvm.c: In function `hvm_create_ioreq_server':
> hvm.c:487:18: error: `bufioreq_pfn' may be used uninitialised in this function
> [-Werror=uninitialized]
> hvm.c:718:30: note: `bufioreq_pfn' was declared here
> ----------------------------------------------------------------------
>
> My code analysis says that gcc is wrong, but initilize the variable
> to prevent the gcc warning.
>
> Reported-by: Ian Murray <murrayie@yahoo.co.uk>
> Signed-off-by: Don Slutz <dslutz@verizon.com>

GCC is correct and there is path through this function where
bufioreq_pfn is used while uninitialised (non-debug build, is_default =
1, handle_bufioreq = 0).

As an aside, the compiler is in a very easy position to spot this.  The
error means that GCC has positively identified a basic block which does
use bufioreq_pfn before it has been initialised.

I observe that the patch has been committed, but it merely hides the
problem and doesn't fix it; hvm_free_ioreq_gmfn(d, bufioreq_pfn) will
still clobber arbitrary memory.

No combination of parameters should be able to cause UB like this.  In
this case, the error handling is incredibly fragile.  All the unmapping
should be based on information in s->{,buf}ioreq which is set upon
successful map, rather than the booleans indicating whether the function
should have set up the mappings.  This allows all the fail$N labels to
collapse into a single, more simple, exit path.

~Andrew