linux-kernel - [PATCH] adjust the spin_lock_ pairs in down family functions (semaphore)

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Fri, 2 Mar 2012 22:37:23 +0800
From:	Dennis Chen <kernel.org.gnu@...il.com>
To:	linux-kernel@...r.kernel.org
Subject: [PATCH] adjust the spin_lock_ pairs in down family functions (semaphore)

Current down family functions use mismatch spin_lock pairs, this will
incur some interrupt state chaos, for example,
down_interruptible --
    spin_lock_irqsave(&sem->lock, flags);              P1
          __down_common--
                spin_unlock_irq(&sem->lock);              P2
               timeout = schedule_timeout(timeout);
               spin_lock_irq(&sem->lock);                  P3

    spin_unlock_irqrestore(&sem->lock, flags);       P4

Suppose 2 kernel thread A and B in an UP system call
down_interruptible to get the semaphore, if the irq is _OFF_ before A
calls, in the section between P2 and P3,  the irq will be turned _ON_,
then B begins to call down_interruptible, it will save a flag
indicating irq is _ON_. So after A finish the path of
down_interruptible, the irq is still _OFF_, but when B wakes up and
finish the path, the irq will be _ON_. Actually, irq should be in on
state before any down_interruptible calling, so
spin_lock_irqsave/irqrestore is not necessary. Given it will make
confusion for the reason of unmatched spin_lock pairs between
down_interruptible and __down_common, so it's reason for the patch.
Any comments?

--- semaphore.bak.c     2012-03-02 21:37:50.510302064 +0800
+++ semaphore.c 2012-03-02 21:46:32.330269122 +0800
@@ -54,12 +54,12 @@
 {
       unsigned long flags;

-       raw_spin_lock_irqsave(&sem->lock, flags);
+       raw_spin_lock_irq(&sem->lock, flags);
       if (likely(sem->count > 0))
               sem->count--;
       else
               __down(sem);
-       raw_spin_unlock_irqrestore(&sem->lock, flags);
+       raw_spin_unlock_irq(&sem->lock, flags);
 }
 EXPORT_SYMBOL(down);

@@ -77,12 +77,12 @@
       unsigned long flags;
       int result = 0;

-       raw_spin_lock_irqsave(&sem->lock, flags);
+       raw_spin_lock_irq(&sem->lock, flags);
       if (likely(sem->count > 0))
               sem->count--;
       else
               result = __down_interruptible(sem);
-       raw_spin_unlock_irqrestore(&sem->lock, flags);
+       raw_spin_unlock_irq(&sem->lock, flags);

       return result;
 }
@@ -103,12 +103,12 @@
       unsigned long flags;
       int result = 0;

-       raw_spin_lock_irqsave(&sem->lock, flags);
+       raw_spin_lock_irq(&sem->lock, flags);
       if (likely(sem->count > 0))
               sem->count--;
       else
               result = __down_killable(sem);
-       raw_spin_unlock_irqrestore(&sem->lock, flags);
+       raw_spin_unlock_irq(&sem->lock, flags);

       return result;
 }
@@ -157,12 +157,12 @@
       unsigned long flags;
       int result = 0;

-       raw_spin_lock_irqsave(&sem->lock, flags);
+       raw_spin_lock_irq(&sem->lock, flags);
       if (likely(sem->count > 0))
               sem->count--;
       else
               result = __down_timeout(sem, jiffies);
-       raw_spin_unlock_irqrestore(&sem->lock, flags);
+       raw_spin_unlock_irq(&sem->lock, flags);

       return result;
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/