Skip to contentSkip to conversation

kvm: irqchip: Break up high order allocations of kvm_irq_routing_table

PATCH New Dan Carpenter 2 participants 2 messages Jun 26, 2015 KVM

lore
Share & tools
lore
#0 lore raw

Hello Joerg Roedel,

The patch e73f61e41f3b: "kvm: irqchip: Break up high order allocations of kvm_irq_routing_table" from May 8, 2015, leads to the following static checker warning:

	arch/x86/kvm/../../../virt/kvm/irqchip.c:238 kvm_set_irq_routing()
	warn: possible memory leak of 'e'

arch/x86/kvm/../../../virt/kvm/irqchip.c
207 for (i = 0; i < nr; ++i) {
208 struct kvm_kernel_irq_routing_entry *e;
209

   210                  r = -ENOMEM;
   211                  e = kzalloc(sizeof(*e), GFP_KERNEL);
   212                  if (!e)
   213                          goto out;
   214  
   215                  r = -EINVAL;
   216                  if (ue->flags)
   217                          goto out;
				^^^^^^^^

Leaked here. Move in front of the allocation?

   218                  r = setup_routing_entry(new, e, ue);
   219                  if (r)
   220                          goto out;
   221                  ++ue;
   222          }
   223  
   224          mutex_lock(&kvm->irq_lock);
   225          old = kvm->irq_routing;
   226          rcu_assign_pointer(kvm->irq_routing, new);
   227          kvm_irq_routing_update(kvm);
   228          mutex_unlock(&kvm->irq_lock);
   229  
   230          synchronize_srcu_expedited(&kvm->irq_srcu);
   231  
   232          new = old;
   233          r = 0;
   234  
   235  out:
   236          free_irq_routing_table(new);
   237  
   238          return r;
   239  }

regards,
dan carpenter

#1 lore raw

PATCH 1/1 kvm: irqchip: Fix possible memory leak in kvm_set_irq_routing()

Hi Dan,

Quoted 8 lines · Dan Carpenter

The patch e73f61e41f3b: "kvm: irqchip: Break up high order allocations of kvm_irq_routing_table" from May 8, 2015, leads to the following static checker warning:

   215                  r = -EINVAL;
   216                  if (ue->flags)
   217                          goto out;
				^^^^^^^^

Leaked here. Move in front of the allocation?

Right, this is a potential leak, thanks for the report. The patch below should fix it:

Quoted 1 line

From 14abe455d04f7208a16237a2f1321fd5e5c5d115 Mon Sep 17 00:00:00 2001

From: Joerg Roedel <[email protected]>
Date: Fri, 26 Jun 2015 18:02:47 +0200
Subject: [PATCH] kvm: irqchip: Fix possible memory leak in
kvm_set_irq_routing()

If ue->flags field is checked after the allocation of the kvm_kernel_irq_routing_entry, it will be leaked if the check succeeds. Do the check before the allocation instead to avoid this leak.

Fixes e73f61e41f3b: "kvm: irqchip: Break up high order allocations of kvm_irq_routing_table"
---
 virt/kvm/irqchip.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
virt/kvm/irqchip.c +4 -3
index 21c1424..239f4ec 100644
@@ -207,14 +207,15 @@ int kvm_set_irq_routing(struct kvm *kvm,
 	for (i = 0; i < nr; ++i) {
 		struct kvm_kernel_irq_routing_entry *e;
 
+		r = -EINVAL;
+		if (ue->flags)
+			goto out;
+
 		r = -ENOMEM;
 		e = kzalloc(sizeof(*e), GFP_KERNEL);
 		if (!e)
 			goto out;
 
-		r = -EINVAL;
-		if (ue->flags)
-			goto out;
 		r = setup_routing_entry(new, e, ue);
 		if (r)
 			goto out;
Signature · 1 line
1.8.4.5