linux-kernel - Re: [PATCH] genirq: Avoid race between cpu hot plug and irq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 05 Sep 2018 16:27:09 -0700
From:   pheragu@...eaurora.org
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     linux-kernel@...r.kernel.org, ckadabi@...eaurora.org,
        tsoni@...eaurora.org, bryanh@...eaurora.org,
        Prasad Sodagudi <psodagud@...eaurora.org>
Subject: Re: [PATCH] genirq: Avoid race between cpu hot plug and irq_desc()
 allocation paths

On 2018-09-05 11:23, Thomas Gleixner wrote:
> On Wed, 5 Sep 2018, Prakruthi Deepak Heragu wrote:
> 
>> One of the cores might have just allocated irq_desc() and other core
>> might be doing irq migration in the hot plug path. In the hot plug 
>> path
>> during the IRQ migration, for_each_active_irq macro is trying to get
>> irqs whose bits are set in allocated_irqs bit map but there is no 
>> return
>> value check after irq_to_desc for desc validity.
> 
> Confused. All parts involved, irq allocation/deallocation and the CPU
> hotplug code take sparse_irq_lock to prevent exavtly that.
> 
Removing the NULL pointer check and adding this sparse_irq_lock
that you suggested will solve this issue. The code looks like
this now. Is this okay?
diff --git a/kernel/irq/cpuhotplug.c b/kernel/irq/cpuhotplug.c
index 9409b55..f2ef76e 100644
--- a/kernel/irq/cpuhotplug.c
+++ b/kernel/irq/cpuhotplug.c
@@ -185,14 +185,10 @@ void irq_migrate_all_off_this_cpu(void)
  {
  	struct irq_desc *desc;
  	unsigned int irq;
-
+	irq_lock_sparse();
  	for_each_active_irq(irq) {
  		bool affinity_broken;
-
  		desc = irq_to_desc(irq);
-		if (!desc)
-			continue;
-
  		raw_spin_lock(&desc->lock);
  		affinity_broken = migrate_one_irq(desc);
  		raw_spin_unlock(&desc->lock);
@@ -202,6 +198,7 @@ void irq_migrate_all_off_this_cpu(void)
  					    irq, smp_processor_id());
  		}
  	}
+	irq_unlock_sparse();
  }

  static void irq_restore_affinity_of_irq(struct irq_desc *desc, unsigned 
int cpu)

> Thanks,
> 
> 	tglx

Thanks,
Prakruthi Deepak Heragu