Skip to contentSkip to conversation

SELinux fixups needed for preemptable RCU from -rt

PATCH Applied Paul E. McKenney 3 participants 5 messages Apr 22, 2008 SELinux security module

lore
Share & tools
lore

Paul E. McKenney

author
#0 lore raw

PATCH 1/1 SELinux fixups needed for preemptable RCU from -rt

Hello!

The attached patch needs to move from -rt to mainline given preemptable RCU. This patch fixes SELinux code that implicitly assumes that disabling preemption prevents an RCU grace period from completing, an assumption that is valid for Classic RCU, but not necessarily for preemptable RCU. Explicit rcu_read_lock() calls are thus added.

Pointed-out-by Paul E. McKenney <[email protected]>

---

 security/selinux/avc.c   |    9 +++++++++
 security/selinux/netif.c |    2 ++
 2 files changed, 11 insertions(+)
security/selinux/avc.c +9 -0
Index: linux-2.6.24.4-rt4/security/selinux/avc.c
===================================================================
@@ -312,6 +312,7 @@ static inline int avc_reclaim_node(void)
 		if (!spin_trylock_irqsave(&avc_cache.slots_lock[hvalue], flags))
 			continue;
 
+		rcu_read_lock();
 		list_for_each_entry(node, &avc_cache.slots[hvalue], list) {
 			if (atomic_dec_and_test(&node->ae.used)) {
 				/* Recently Unused */

@@ -319,11 +320,13 @@ static inline int avc_reclaim_node(void)
 				avc_cache_stats_incr(reclaims);
 				ecx++;
 				if (ecx >= AVC_CACHE_RECLAIM) {
+					rcu_read_unlock();
 					spin_unlock_irqrestore(&avc_cache.slots_lock[hvalue], flags);
 					goto out;
 				}
 			}
 		}
+		rcu_read_unlock();
 		spin_unlock_irqrestore(&avc_cache.slots_lock[hvalue], flags);
 	}
 out:

@@ -807,8 +810,14 @@ int avc_ss_reset(u32 seqno)
 
 	for (i = 0; i < AVC_CACHE_SLOTS; i++) {
 		spin_lock_irqsave(&avc_cache.slots_lock[i], flag);
+		/*
+		 * On -rt the outer spinlock does not prevent RCU
+		 * from being performed:
+		 */
+		rcu_read_lock();
 		list_for_each_entry(node, &avc_cache.slots[i], list)
 			avc_node_delete(node);
+		rcu_read_unlock();
 		spin_unlock_irqrestore(&avc_cache.slots_lock[i], flag);
 	}
security/selinux/netif.c +2 -0
Index: linux-2.6.24.4-rt4/security/selinux/netif.c
===================================================================
@@ -210,6 +210,7 @@ static void sel_netif_flush(void)
 {
 	int idx;
 
+	rcu_read_lock();
 	spin_lock_bh(&sel_netif_lock);
 	for (idx = 0; idx < SEL_NETIF_HASH_SIZE; idx++) {
 		struct sel_netif *netif;

@@ -218,6 +219,7 @@ static void sel_netif_flush(void)
 			sel_netif_destroy(netif);
 	}
 	spin_unlock_bh(&sel_netif_lock);
+	rcu_read_unlock();
 }
 
 static int sel_netif_avc_callback(u32 event, u32 ssid, u32 tsid,
#1 lore raw

security/selinux/avc.c int avc_ss_reset(u32 seqno) · line 815

Quoted 7 lines · Paul E. McKenney
security/selinux/avc.c +3 -0
@@ -807,8 +810,14 @@ int avc_ss_reset(u32 seqno)
 
 	for (i = 0; i < AVC_CACHE_SLOTS; i++) {
 		spin_lock_irqsave(&avc_cache.slots_lock[i], flag);
+		/*
+		 * On -rt the outer spinlock does not prevent RCU
+		 * from being performed:

I would suggest to change this comment to "With preemptible RCU" from "On -rt".

-- Steve

security/selinux/avc.c int avc_ss_reset(u32 seqno) · line 823

Quoted 8 lines
security/selinux/avc.c +3 -0
+		 */
+		rcu_read_lock();
 		list_for_each_entry(node, &avc_cache.slots[i], list)
 			avc_node_delete(node);
+		rcu_read_unlock();
 		spin_unlock_irqrestore(&avc_cache.slots_lock[i], flag);
 	}

5A

author
#2 lore raw
Quoted 4 lines · Steven Rostedt
Quoted 2 lines · Paul E. McKenney
(quoted diff) +0 -0
@@ -807,8 +810,14 @@ int avc_ss_reset(u32 seqno)
Quoted 8 lines

Review int avc_ss_reset(u32 seqno) · line 815

Quoted 5 lines
(quoted diff) +3 -0
 	for (i = 0; i < AVC_CACHE_SLOTS; i++) {
 		spin_lock_irqsave(&avc_cache.slots_lock[i], flag);
+		/*
+		 * On -rt the outer spinlock does not prevent RCU
+		 * from being performed:

I would suggest to change this comment to "With preemptible RCU" from "On -rt".

Good point... How about the following?

Quoted 9 lines

-- Steve

Review int avc_ss_reset(u32 seqno) · line 822

Quoted 7 lines
(quoted diff) +3 -0
+		 */
+		rcu_read_lock();
 		list_for_each_entry(node, &avc_cache.slots[i], list)
 			avc_node_delete(node);
+		rcu_read_unlock();
 		spin_unlock_irqrestore(&avc_cache.slots_lock[i], flag);
 	}
Signed-off-by Paul E. McKenney <[email protected]> (comment change)
---
 security/selinux/avc.c   |    9 +++++++++
 security/selinux/netif.c |    2 ++
 2 files changed, 11 insertions(+)
security/selinux/avc.c +9 -0
Index: linux-2.6.24.4-rt4/security/selinux/avc.c
===================================================================
@@ -312,6 +312,7 @@ static inline int avc_reclaim_node(void)
 		if (!spin_trylock_irqsave(&avc_cache.slots_lock[hvalue], flags))
 			continue;
 
+		rcu_read_lock();
 		list_for_each_entry(node, &avc_cache.slots[hvalue], list) {
 			if (atomic_dec_and_test(&node->ae.used)) {
 				/* Recently Unused */

@@ -319,11 +320,13 @@ static inline int avc_reclaim_node(void)
 				avc_cache_stats_incr(reclaims);
 				ecx++;
 				if (ecx >= AVC_CACHE_RECLAIM) {
+					rcu_read_unlock();
 					spin_unlock_irqrestore(&avc_cache.slots_lock[hvalue], flags);
 					goto out;
 				}
 			}
 		}
+		rcu_read_unlock();
 		spin_unlock_irqrestore(&avc_cache.slots_lock[hvalue], flags);
 	}
 out:

@@ -807,8 +810,14 @@ int avc_ss_reset(u32 seqno)
 
 	for (i = 0; i < AVC_CACHE_SLOTS; i++) {
 		spin_lock_irqsave(&avc_cache.slots_lock[i], flag);
+		/*
+		 * With preemptable RCU, the outer spinlock does not
+		 * prevent RCU grace periods from ending.
+		 */
+		rcu_read_lock();
 		list_for_each_entry(node, &avc_cache.slots[i], list)
 			avc_node_delete(node);
+		rcu_read_unlock();
 		spin_unlock_irqrestore(&avc_cache.slots_lock[i], flag);
 	}
security/selinux/netif.c +2 -0
Index: linux-2.6.24.4-rt4/security/selinux/netif.c
===================================================================
@@ -210,6 +210,7 @@ static void sel_netif_flush(void)
 {
 	int idx;
 
+	rcu_read_lock();
 	spin_lock_bh(&sel_netif_lock);
 	for (idx = 0; idx < SEL_NETIF_HASH_SIZE; idx++) {
 		struct sel_netif *netif;

@@ -218,6 +219,7 @@ static void sel_netif_flush(void)
 			sel_netif_destroy(netif);
 	}
 	spin_unlock_bh(&sel_netif_lock);
+	rcu_read_unlock();
 }
 
 static int sel_netif_avc_callback(u32 event, u32 ssid, u32 tsid,
Acked-by
#3 lore raw
Quoted 14 lines · Paul E. McKenney
Quoted 4 lines · Steven Rostedt
Quoted 2 lines · Paul E. McKenney
(quoted diff) +0 -0
@@ -807,8 +810,14 @@ int avc_ss_reset(u32 seqno)
Quoted 8 lines

security/selinux/avc.c int avc_ss_reset(u32 seqno)

Quoted 5 lines
security/selinux/avc.c +3 -0
 	for (i = 0; i < AVC_CACHE_SLOTS; i++) {
 		spin_lock_irqsave(&avc_cache.slots_lock[i], flag);
+		/*
+		 * On -rt the outer spinlock does not prevent RCU
+		 * from being performed:

I would suggest to change this comment to "With preemptible RCU" from "On -rt".

Quoted 1 line

Good point... How about the following?

Acked-by Steven Rostedt <[email protected]>

-- Steve

Applied
#4 lore raw
In reply to Paul E. McKenney

Thanks, applied to

git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-2.6.git#for-linus

Quoted 73 lines
Signed-off-by Paul E. McKenney <[email protected]> (comment change)
---
 security/selinux/avc.c   |    9 +++++++++
 security/selinux/netif.c |    2 ++
 2 files changed, 11 insertions(+)

Index: linux-2.6.24.4-rt4/security/selinux/avc.c
===================================================================

security/selinux/avc.c +9 -0
@@ -312,6 +312,7 @@ static inline int avc_reclaim_node(void)
 		if (!spin_trylock_irqsave(&avc_cache.slots_lock[hvalue], flags))
 			continue;
 
+		rcu_read_lock();
 		list_for_each_entry(node, &avc_cache.slots[hvalue], list) {
 			if (atomic_dec_and_test(&node->ae.used)) {
 				/* Recently Unused */

@@ -319,11 +320,13 @@ static inline int avc_reclaim_node(void)
 				avc_cache_stats_incr(reclaims);
 				ecx++;
 				if (ecx >= AVC_CACHE_RECLAIM) {
+					rcu_read_unlock();
 					spin_unlock_irqrestore(&avc_cache.slots_lock[hvalue], flags);
 					goto out;
 				}
 			}
 		}
+		rcu_read_unlock();
 		spin_unlock_irqrestore(&avc_cache.slots_lock[hvalue], flags);
 	}
 out:

@@ -807,8 +810,14 @@ int avc_ss_reset(u32 seqno)
 
 	for (i = 0; i < AVC_CACHE_SLOTS; i++) {
 		spin_lock_irqsave(&avc_cache.slots_lock[i], flag);
+		/*
+		 * With preemptable RCU, the outer spinlock does not
+		 * prevent RCU grace periods from ending.
+		 */
+		rcu_read_lock();
 		list_for_each_entry(node, &avc_cache.slots[i], list)
 			avc_node_delete(node);
+		rcu_read_unlock();
 		spin_unlock_irqrestore(&avc_cache.slots_lock[i], flag);
 	}

Index: linux-2.6.24.4-rt4/security/selinux/netif.c
===================================================================

security/selinux/netif.c +2 -1
@@ -210,6 +210,7 @@ static void sel_netif_flush(void)
 {
 	int idx;
 
+	rcu_read_lock();
 	spin_lock_bh(&sel_netif_lock);
 	for (idx = 0; idx < SEL_NETIF_HASH_SIZE; idx++) {
 		struct sel_netif *netif;

@@ -218,6 +219,7 @@ static void sel_netif_flush(void)
 			sel_netif_destroy(netif);
 	}
 	spin_unlock_bh(&sel_netif_lock);
+	rcu_read_unlock();
 }
 
 static int sel_netif_avc_callback(u32 event, u32 ssid, u32 tsid,
--

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 2 lines
James Morris
<[email protected]>