Xen devel - Sep 2013 - [PATCH] x86/mm: Fix possible increment of uninitialised variable

Discovered by Coverity, CID 1056101

When taking the continue branch on the first iteration of the loop, gfn would
indeed be uninitialised when incremented.  However, as gfn is unconditionally
constructed from i{1..4} before use in the loop body, having it incremented in
the loop header is useless.

Therefore, simply remove it.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>

---

I have compile tested this but not functionally tested it.  It is fairly
obvious from the code that it was simply wrong in the first place.
---
 xen/arch/x86/mm/p2m-pt.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index 302b621..a1d5650 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -691,7 +691,7 @@ static void p2m_change_type_global(struct p2m_domain *p2m,
                 l1mfn = _mfn(l2e_get_pfn(l2e[i2]));
                 l1e = map_domain_page(mfn_x(l1mfn));
 
-                for ( i1 = 0; i1 < L1_PAGETABLE_ENTRIES; i1++, gfn++ )
+                for ( i1 = 0; i1 < L1_PAGETABLE_ENTRIES; i1++ )
                 {
                     flags = l1e_get_flags(l1e[i1]);
                     if ( p2m_flags_to_type(flags) != ot )
-- 
1.7.10.4

>>> On 09.09.13 at 19:43, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
> Discovered by Coverity, CID 1056101
> 
> When taking the continue branch on the first iteration of the loop, gfn 
> would
> indeed be uninitialised when incremented.  However, as gfn is 
> unconditionally
> constructed from i{1..4} before use in the loop body, having it incremented
> in
> the loop header is useless.
> 
> Therefore, simply remove it.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
> I have compile tested this but not functionally tested it.  It is fairly
> obvious from the code that it was simply wrong in the first place.
Indeed, or else the similar L2 and L3 loops would also have needed
some form of increment.

Jan

At 18:43 +0100 on 09 Sep (1378752220), Andrew Cooper
wrote:
> Discovered by Coverity, CID 1056101
> 
> When taking the continue branch on the first iteration of the loop, gfn
would
> indeed be uninitialised when incremented.  However, as gfn is
unconditionally
> constructed from i{1..4} before use in the loop body, having it incremented
in
> the loop header is useless.
> 
> Therefore, simply remove it.
Acked + applied, thanks.

Tim.