linux-kernel - Re: On some configs, sparse spinlock balance checking is broken

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <adavej5k6ld.fsf@cisco.com>
Date:	Wed, 17 Jan 2007 07:37:02 -0800
From:	Roland Dreier <rdreier@...co.com>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: On some configs, sparse spinlock balance checking is broken

 > i think the right way to fix it might be to define a _spin_unlock() 
 > within those #ifdef branches, and then to define spin_lock as:
 > 
 > static inline void spin_lock(spinlock_t *lock) __acquires(lock)

I tried a similar approach, but what got me was that sparse doesn't
pay attention to the "__acquires()" annotation there.  However I now
realized that putting "__acquire()" inside the implementation of the
function (which sparse can see for inline functions) actually works.

And actually the lock stuff is OK, since it's not inlined -- it's the
unlock stuff that goes directly to the __raw versions.  But something
like the following works for me; does it look OK to you?

---

diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
index 94b767d..8ec4142 100644
--- a/include/linux/spinlock.h
+++ b/include/linux/spinlock.h
@@ -228,15 +228,45 @@ do {								\
 # define read_unlock_irq(lock)		_read_unlock_irq(lock)
 # define write_unlock_irq(lock)		_write_unlock_irq(lock)
 #else
-# define spin_unlock(lock)		__raw_spin_unlock(&(lock)->raw_lock)
-# define read_unlock(lock)		__raw_read_unlock(&(lock)->raw_lock)
-# define write_unlock(lock)		__raw_write_unlock(&(lock)->raw_lock)
-# define spin_unlock_irq(lock) \
-    do { __raw_spin_unlock(&(lock)->raw_lock); local_irq_enable(); } while (0)
-# define read_unlock_irq(lock) \
-    do { __raw_read_unlock(&(lock)->raw_lock); local_irq_enable(); } while (0)
-# define write_unlock_irq(lock) \
-    do { __raw_write_unlock(&(lock)->raw_lock); local_irq_enable(); } while (0)
+static inline void spin_unlock(spinlock_t *lock)
+{
+	__release(lock);
+	__raw_spin_unlock(&(lock)->raw_lock);
+}
+
+static inline void read_unlock(rwlock_t *lock)
+{
+	__release(lock);
+	__raw_read_unlock(&(lock)->raw_lock);
+}
+
+static inline void write_unlock(rwlock_t *lock)
+{
+	__release(lock);
+	__raw_write_unlock(&(lock)->raw_lock);
+}
+
+static inline void spin_unlock_irq(spinlock_t *lock)
+{
+	__release(lock);
+	__raw_spin_unlock(&(lock)->raw_lock);
+	local_irq_enable();
+}
+
+static inline void read_unlock_irq(rwlock_t *lock)
+{
+	__release(lock);
+	__raw_read_unlock(&(lock)->raw_lock);
+	local_irq_enable();
+}
+
+static inline void write_unlock_irq(rwlock_t *lock)
+{
+	__release(lock);
+	__raw_write_unlock(&(lock)->raw_lock);
+	local_irq_enable();
+}
+
 #endif
 
 #define spin_unlock_irqrestore(lock, flags) \
-
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/