Re: [Xen-devel] [PATCH] libxc: fix uninitialised usage of rc in meminit_hvm

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Tue, 2016-02-02 at 12:37 +0000, Wei Liu wrote:
> On Tue, Feb 02, 2016 at 12:33:20PM +0100, Roger Pau Monne wrote:
> > From: Roger Pau Monne <royger@xxxxxxxxxxx>
> > 
> > Due to the HVMlite changes there's a chance that the value in rc is
> > checked
> > without being initialised. Fix this by initialising it to 0.
> > 
> > Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
> > Reported-by: Olaf Hering <olaf@xxxxxxxxx>
> 
> Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>

This is CIDÂ1351229, I think?

** CID 1351229:ÂÂUninitialized variablesÂÂ(UNINIT)
> /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
>Â
>Â
> ________________________________________________________________________________________________________
> *** CID 1351229:ÂÂUninitialized variablesÂÂ(UNINIT)
> /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
> 1437ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂcur_pages = 0xc0;
> 1438ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂstat_normal_pages += 0xc0;
> 1439ÂÂÂÂÂÂÂÂÂÂÂÂÂ}
> 1440ÂÂÂÂÂÂÂÂÂÂÂÂÂelse
> 1441ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂcur_pages = vmemranges[vmemid].start >> PAGE_SHIFT;
> 1442ÂÂÂÂÂ
> >>>ÂÂÂÂÂCID 1351229:ÂÂUninitialized variablesÂÂ(UNINIT)
> >>>ÂÂÂÂÂUsing uninitialized value "rc".
> 1443ÂÂÂÂÂÂÂÂÂÂÂÂÂwhile ( (rc == 0) && (end_pages > cur_pages) )
> 1444ÂÂÂÂÂÂÂÂÂÂÂÂÂ{
> 1445ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ/* Clip count to maximum 1GB extent. */
> 1446ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂunsigned long count = end_pages - cur_pages;
> 1447ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂunsigned long max_pages = SUPERPAGE_1GB_NR_PFNS;
> 1448 Â Â

Note that this while loop ends with:
ÂÂÂÂÂÂÂÂif ( rc != 0 )
ÂÂÂÂÂÂÂÂÂÂÂÂbreak;
and there are no continue statements.

Therefore I wonder if we would be better off removing the rc == 0 part of
the loop condition?

The issue with this patch is the usual one that it will hide other
unintentional uses of rc before it is set to a good value.

This issue was exposed by a prior "rc = xc_domain_populate_physmap_exact"
becoming conditional onÂdevice_model. What is also concerning is the lack
of error checking on that call -- is it really ok to just barrel on under
these circumstance?

Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel