Skip to contentSkip to conversation

: Protect against BHs in xfrm_user_policy()

IPSEC Patrick McHardy 3 participants 6 messages Apr 3, 2005 – Apr 4, 2005

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Patrick McHardy

author
#0 lore raw

xfrm_user_policy() is called from ip_setsockopt with enabled BHs, so it needs to protect against them when grabbing xfrm_km_lock.

# This is a BitKeeper generated diff -Nru style patch. #
# ChangeSet
# 2005/04/03 17:36:10+02:00 [email protected]
# [IPSEC]: Protect against BHs in xfrm_user_policy()
#
# Signed-off-by: Patrick McHardy <[email protected]>
#

# net/xfrm/xfrm_state.c
#   2005/04/03 17:36:00+02:00 [email protected] +2 -2
#   [IPSEC]: Protect against BHs in xfrm_user_policy()
#   
#   Signed-off-by: Patrick McHardy <[email protected]>
# 
diff -Nru a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
--- a/net/xfrm/xfrm_state.c	2005-04-03 18:04:38 +02:00
+++ b/net/xfrm/xfrm_state.c	2005-04-03 18:04:38 +02:00
@@ -878,14 +878,14 @@
 		goto out;

err = -EINVAL;
- read_lock(&xfrm_km_lock);
+ read_lock_bh(&xfrm_km_lock);
list_for_each_entry(km, &xfrm_km_list, list) {
pol = km->compile_policy(sk->sk_family, optname, data,
optlen, &err);
if (err >= 0)
break;
}
- read_unlock(&xfrm_km_lock);
+ read_unlock_bh(&xfrm_km_lock);

if (err >= 0) {
xfrm_sk_policy_insert(sk, err, pol);

#1 lore raw
Quoted 8 lines · Patrick McHardy

# This is a BitKeeper generated diff -Nru style patch. #
# ChangeSet
# 2005/04/03 17:36:10+02:00 [email protected]
# [IPSEC]: Protect against BHs in xfrm_user_policy()
#
# Signed-off-by: Patrick McHardy <[email protected]>

Looks good.

Signed-off-by Herbert Xu <[email protected]>

We want the same thing for km_query, no?

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
author
#2 lore raw
Quoted 1 line · Herbert Xu

We want the same thing for km_query, no?

In all other places were BHs are not explicitly disabled but need to be they are already disabled by the caller, so I left them as they are.

Regards
Patrick

#3 lore raw
Quoted 6 lines · Patrick McHardy
Quoted 1 line · Herbert Xu

We want the same thing for km_query, no?

In all other places were BHs are not explicitly disabled but need to be they are already disabled by the caller, so I left them as they are.

Yes you're right. I missed the spin_lock_bh in xfrm_state_find.

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#4 lore raw
In reply to Herbert Xu
Quoted 10 lines · herbert
Quoted 8 lines · Patrick McHardy

# This is a BitKeeper generated diff -Nru style patch. #
# ChangeSet
# 2005/04/03 17:36:10+02:00 [email protected]
# [IPSEC]: Protect against BHs in xfrm_user_policy()
#
# Signed-off-by: Patrick McHardy <[email protected]>

Quoted 3 lines

Looks good.

Signed-off-by Herbert Xu <[email protected]>

Actually, I now think this patch is unnecessary for mainline.

The read_lock()'s only need to be protected from the write_lock()'s.

Since all the write_lock()'s are made in process context, we don't need to disable BH on the read_lock()'s.

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#5 lore raw
Quoted 4 lines · On Mon, 4 Apr 2005 21:55:08 +1000 Herbert Xu <[email protected]>…

The read_lock()'s only need to be protected from the write_lock()'s.

Since all the write_lock()'s are made in process context, we don't need to disable BH on the read_lock()'s.

This is correct.

It's actually a common technique, only disable IRQ or BH in the write_locks.