* [PATCH net-next] xfrm: fix RCU bugs
@ 2012-08-19 10:31 Eric Dumazet
2012-08-20 4:40 ` Fan Du
2012-08-23 5:40 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Eric Dumazet @ 2012-08-19 10:31 UTC (permalink / raw)
To: David Miller; +Cc: netdev, fan.du, fengguang.wu, Priyanka Jain
From: Eric Dumazet <edumazet@google.com>
This patch reverts commit 56892261ed1a (xfrm: Use rcu_dereference_bh to
deference pointer protected by rcu_read_lock_bh), and fixes bugs
introduced in commit 418a99ac6ad ( Replace rwlock on xfrm_policy_afinfo
with rcu )
1) We properly use RCU variant in this file, not a mix of RCU/RCU_BH
2) We must defer some writes after the synchronize_rcu() call or a reader
can crash dereferencing NULL pointer.
3) Now we use the xfrm_policy_afinfo_lock spinlock only from process
context, we no longer need to block BH in xfrm_policy_register_afinfo()
and xfrm_policy_unregister_afinfo()
4) Can use RCU_INIT_POINTER() instead of rcu_assign_pointer() in
xfrm_policy_unregister_afinfo()
5) Remove a forward inline declaration (xfrm_policy_put_afinfo()),
and also move xfrm_policy_get_afinfo() declaration.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Fan Du <fan.du@windriver.com>
Cc: Priyanka Jain <Priyanka.Jain@freescale.com>
---
net/xfrm/xfrm_policy.c | 76 ++++++++++++++++++++-------------------
1 file changed, 39 insertions(+), 37 deletions(-)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 6405764..e52f50f 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -48,8 +48,6 @@ static struct xfrm_policy_afinfo __rcu *xfrm_policy_afinfo[NPROTO]
static struct kmem_cache *xfrm_dst_cache __read_mostly;
-static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family);
-static inline void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo);
static void xfrm_init_pmtu(struct dst_entry *dst);
static int stale_bundle(struct dst_entry *dst);
static int xfrm_bundle_ok(struct xfrm_dst *xdst);
@@ -96,6 +94,24 @@ bool xfrm_selector_match(const struct xfrm_selector *sel, const struct flowi *fl
return false;
}
+static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family)
+{
+ struct xfrm_policy_afinfo *afinfo;
+
+ if (unlikely(family >= NPROTO))
+ return NULL;
+ rcu_read_lock();
+ afinfo = rcu_dereference(xfrm_policy_afinfo[family]);
+ if (unlikely(!afinfo))
+ rcu_read_unlock();
+ return afinfo;
+}
+
+static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo)
+{
+ rcu_read_unlock();
+}
+
static inline struct dst_entry *__xfrm_dst_lookup(struct net *net, int tos,
const xfrm_address_t *saddr,
const xfrm_address_t *daddr,
@@ -2419,7 +2435,7 @@ int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo)
return -EINVAL;
if (unlikely(afinfo->family >= NPROTO))
return -EAFNOSUPPORT;
- spin_lock_bh(&xfrm_policy_afinfo_lock);
+ spin_lock(&xfrm_policy_afinfo_lock);
if (unlikely(xfrm_policy_afinfo[afinfo->family] != NULL))
err = -ENOBUFS;
else {
@@ -2442,7 +2458,7 @@ int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo)
afinfo->garbage_collect = xfrm_garbage_collect_deferred;
rcu_assign_pointer(xfrm_policy_afinfo[afinfo->family], afinfo);
}
- spin_unlock_bh(&xfrm_policy_afinfo_lock);
+ spin_unlock(&xfrm_policy_afinfo_lock);
rtnl_lock();
for_each_net(net) {
@@ -2475,23 +2491,26 @@ int xfrm_policy_unregister_afinfo(struct xfrm_policy_afinfo *afinfo)
return -EINVAL;
if (unlikely(afinfo->family >= NPROTO))
return -EAFNOSUPPORT;
- spin_lock_bh(&xfrm_policy_afinfo_lock);
+ spin_lock(&xfrm_policy_afinfo_lock);
if (likely(xfrm_policy_afinfo[afinfo->family] != NULL)) {
if (unlikely(xfrm_policy_afinfo[afinfo->family] != afinfo))
err = -EINVAL;
- else {
- struct dst_ops *dst_ops = afinfo->dst_ops;
- rcu_assign_pointer(xfrm_policy_afinfo[afinfo->family],
- NULL);
- dst_ops->kmem_cachep = NULL;
- dst_ops->check = NULL;
- dst_ops->negative_advice = NULL;
- dst_ops->link_failure = NULL;
- afinfo->garbage_collect = NULL;
- }
+ else
+ RCU_INIT_POINTER(xfrm_policy_afinfo[afinfo->family],
+ NULL);
+ }
+ spin_unlock(&xfrm_policy_afinfo_lock);
+ if (!err) {
+ struct dst_ops *dst_ops = afinfo->dst_ops;
+
+ synchronize_rcu();
+
+ dst_ops->kmem_cachep = NULL;
+ dst_ops->check = NULL;
+ dst_ops->negative_advice = NULL;
+ dst_ops->link_failure = NULL;
+ afinfo->garbage_collect = NULL;
}
- spin_unlock_bh(&xfrm_policy_afinfo_lock);
- synchronize_rcu();
return err;
}
EXPORT_SYMBOL(xfrm_policy_unregister_afinfo);
@@ -2500,32 +2519,15 @@ static void __net_init xfrm_dst_ops_init(struct net *net)
{
struct xfrm_policy_afinfo *afinfo;
- rcu_read_lock_bh();
- afinfo = rcu_dereference_bh(xfrm_policy_afinfo[AF_INET]);
+ rcu_read_lock();
+ afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET]);
if (afinfo)
net->xfrm.xfrm4_dst_ops = *afinfo->dst_ops;
#if IS_ENABLED(CONFIG_IPV6)
- afinfo = rcu_dereference_bh(xfrm_policy_afinfo[AF_INET6]);
+ afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET6]);
if (afinfo)
net->xfrm.xfrm6_dst_ops = *afinfo->dst_ops;
#endif
- rcu_read_unlock_bh();
-}
-
-static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family)
-{
- struct xfrm_policy_afinfo *afinfo;
- if (unlikely(family >= NPROTO))
- return NULL;
- rcu_read_lock();
- afinfo = rcu_dereference(xfrm_policy_afinfo[family]);
- if (unlikely(!afinfo))
- rcu_read_unlock();
- return afinfo;
-}
-
-static inline void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo)
-{
rcu_read_unlock();
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net-next] xfrm: fix RCU bugs
2012-08-19 10:31 [PATCH net-next] xfrm: fix RCU bugs Eric Dumazet
@ 2012-08-20 4:40 ` Fan Du
2012-08-20 5:33 ` Eric Dumazet
2012-08-23 5:40 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Fan Du @ 2012-08-20 4:40 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, fengguang.wu, Priyanka Jain
Hi Eric
Please correct me if I'm wrong about below comments.
On 2012年08月19日 18:31, Eric Dumazet wrote:
> From: Eric Dumazet<edumazet@google.com>
>
> This patch reverts commit 56892261ed1a (xfrm: Use rcu_dereference_bh to
> deference pointer protected by rcu_read_lock_bh), and fixes bugs
> introduced in commit 418a99ac6ad ( Replace rwlock on xfrm_policy_afinfo
> with rcu )
>
> 1) We properly use RCU variant in this file, not a mix of RCU/RCU_BH
>
> 2) We must defer some writes after the synchronize_rcu() call or a reader
> can crash dereferencing NULL pointer.
Not exactly.
net/ipv4/xfrm4_policy.c
static void __exit xfrm4_policy_fini(void)
-> xfrm_policy_unregister_afinfo
IMHO, ip stack can never be compiled as module, so is xfrm4_policy_fini
freed up after system bootup? which means xfrm4_policy_fini can never be
called.
so an dereferencing NULL pointer by a reader could not happen.
>
> 3) Now we use the xfrm_policy_afinfo_lock spinlock only from process
> context, we no longer need to block BH in xfrm_policy_register_afinfo()
> and xfrm_policy_unregister_afinfo()
>
I don't think it's related to what kinds of locks we are using.
we call xfrm_policy_register_afinfo in process context, but actually
what xfrm_policy_afinfo_lock protected can be used in soft irq context.
that's why xx_bh is used in:
e959d812 " [XFRM]: fix incorrect xfrm_policy_afinfo_lock use"
Is such scenario still valid?
> 4) Can use RCU_INIT_POINTER() instead of rcu_assign_pointer() in
> xfrm_policy_unregister_afinfo()
>
> 5) Remove a forward inline declaration (xfrm_policy_put_afinfo()),
> and also move xfrm_policy_get_afinfo() declaration.
>
> Signed-off-by: Eric Dumazet<edumazet@google.com>
> Cc: Fan Du<fan.du@windriver.com>
> Cc: Priyanka Jain<Priyanka.Jain@freescale.com>
> ---
> net/xfrm/xfrm_policy.c | 76 ++++++++++++++++++++-------------------
> 1 file changed, 39 insertions(+), 37 deletions(-)
>
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index 6405764..e52f50f 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -48,8 +48,6 @@ static struct xfrm_policy_afinfo __rcu *xfrm_policy_afinfo[NPROTO]
>
> static struct kmem_cache *xfrm_dst_cache __read_mostly;
>
> -static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family);
> -static inline void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo);
> static void xfrm_init_pmtu(struct dst_entry *dst);
> static int stale_bundle(struct dst_entry *dst);
> static int xfrm_bundle_ok(struct xfrm_dst *xdst);
> @@ -96,6 +94,24 @@ bool xfrm_selector_match(const struct xfrm_selector *sel, const struct flowi *fl
> return false;
> }
>
> +static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family)
> +{
> + struct xfrm_policy_afinfo *afinfo;
> +
> + if (unlikely(family>= NPROTO))
> + return NULL;
> + rcu_read_lock();
> + afinfo = rcu_dereference(xfrm_policy_afinfo[family]);
> + if (unlikely(!afinfo))
> + rcu_read_unlock();
> + return afinfo;
> +}
> +
> +static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo)
> +{
> + rcu_read_unlock();
> +}
> +
> static inline struct dst_entry *__xfrm_dst_lookup(struct net *net, int tos,
> const xfrm_address_t *saddr,
> const xfrm_address_t *daddr,
> @@ -2419,7 +2435,7 @@ int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo)
> return -EINVAL;
> if (unlikely(afinfo->family>= NPROTO))
> return -EAFNOSUPPORT;
> - spin_lock_bh(&xfrm_policy_afinfo_lock);
> + spin_lock(&xfrm_policy_afinfo_lock);
> if (unlikely(xfrm_policy_afinfo[afinfo->family] != NULL))
> err = -ENOBUFS;
> else {
> @@ -2442,7 +2458,7 @@ int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo)
> afinfo->garbage_collect = xfrm_garbage_collect_deferred;
> rcu_assign_pointer(xfrm_policy_afinfo[afinfo->family], afinfo);
> }
> - spin_unlock_bh(&xfrm_policy_afinfo_lock);
> + spin_unlock(&xfrm_policy_afinfo_lock);
>
> rtnl_lock();
> for_each_net(net) {
> @@ -2475,23 +2491,26 @@ int xfrm_policy_unregister_afinfo(struct xfrm_policy_afinfo *afinfo)
> return -EINVAL;
> if (unlikely(afinfo->family>= NPROTO))
> return -EAFNOSUPPORT;
> - spin_lock_bh(&xfrm_policy_afinfo_lock);
> + spin_lock(&xfrm_policy_afinfo_lock);
> if (likely(xfrm_policy_afinfo[afinfo->family] != NULL)) {
> if (unlikely(xfrm_policy_afinfo[afinfo->family] != afinfo))
> err = -EINVAL;
> - else {
> - struct dst_ops *dst_ops = afinfo->dst_ops;
> - rcu_assign_pointer(xfrm_policy_afinfo[afinfo->family],
> - NULL);
> - dst_ops->kmem_cachep = NULL;
> - dst_ops->check = NULL;
> - dst_ops->negative_advice = NULL;
> - dst_ops->link_failure = NULL;
> - afinfo->garbage_collect = NULL;
> - }
> + else
> + RCU_INIT_POINTER(xfrm_policy_afinfo[afinfo->family],
> + NULL);
> + }
> + spin_unlock(&xfrm_policy_afinfo_lock);
> + if (!err) {
> + struct dst_ops *dst_ops = afinfo->dst_ops;
> +
> + synchronize_rcu();
> +
> + dst_ops->kmem_cachep = NULL;
> + dst_ops->check = NULL;
> + dst_ops->negative_advice = NULL;
> + dst_ops->link_failure = NULL;
> + afinfo->garbage_collect = NULL;
> }
> - spin_unlock_bh(&xfrm_policy_afinfo_lock);
> - synchronize_rcu();
> return err;
> }
> EXPORT_SYMBOL(xfrm_policy_unregister_afinfo);
> @@ -2500,32 +2519,15 @@ static void __net_init xfrm_dst_ops_init(struct net *net)
> {
> struct xfrm_policy_afinfo *afinfo;
>
> - rcu_read_lock_bh();
> - afinfo = rcu_dereference_bh(xfrm_policy_afinfo[AF_INET]);
> + rcu_read_lock();
> + afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET]);
> if (afinfo)
> net->xfrm.xfrm4_dst_ops = *afinfo->dst_ops;
> #if IS_ENABLED(CONFIG_IPV6)
> - afinfo = rcu_dereference_bh(xfrm_policy_afinfo[AF_INET6]);
> + afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET6]);
> if (afinfo)
> net->xfrm.xfrm6_dst_ops = *afinfo->dst_ops;
> #endif
> - rcu_read_unlock_bh();
> -}
> -
> -static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family)
> -{
> - struct xfrm_policy_afinfo *afinfo;
> - if (unlikely(family>= NPROTO))
> - return NULL;
> - rcu_read_lock();
> - afinfo = rcu_dereference(xfrm_policy_afinfo[family]);
> - if (unlikely(!afinfo))
> - rcu_read_unlock();
> - return afinfo;
> -}
> -
> -static inline void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo)
> -{
> rcu_read_unlock();
> }
>
>
>
>
--
Love each day!
--fan
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next] xfrm: fix RCU bugs
2012-08-20 4:40 ` Fan Du
@ 2012-08-20 5:33 ` Eric Dumazet
2012-08-20 6:33 ` Fan Du
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2012-08-20 5:33 UTC (permalink / raw)
To: Fan Du; +Cc: David Miller, netdev, fengguang.wu, Priyanka Jain
On Mon, 2012-08-20 at 12:40 +0800, Fan Du wrote:
> Hi Eric
>
> Please correct me if I'm wrong about below comments.
>
> On 2012年08月19日 18:31, Eric Dumazet wrote:
> > From: Eric Dumazet<edumazet@google.com>
> >
> > This patch reverts commit 56892261ed1a (xfrm: Use rcu_dereference_bh to
> > deference pointer protected by rcu_read_lock_bh), and fixes bugs
> > introduced in commit 418a99ac6ad ( Replace rwlock on xfrm_policy_afinfo
> > with rcu )
> >
> > 1) We properly use RCU variant in this file, not a mix of RCU/RCU_BH
> >
> > 2) We must defer some writes after the synchronize_rcu() call or a reader
> > can crash dereferencing NULL pointer.
>
> Not exactly.
>
> net/ipv4/xfrm4_policy.c
> static void __exit xfrm4_policy_fini(void)
> -> xfrm_policy_unregister_afinfo
>
> IMHO, ip stack can never be compiled as module, so is xfrm4_policy_fini
> freed up after system bootup? which means xfrm4_policy_fini can never be
> called.
>
> so an dereferencing NULL pointer by a reader could not happen.
>
Last famous words.
Anyway xfrm_policy_unregister_afinfo() is also called from
xfrm6_policy_fini(), and IPv6 is a module. The day we can rmmod it,
we uncover this bug.
RCU is complex (most people dont get it right, thats the truth),
and we should make it rock solid, or I can guarantee you
many patch attempts from future readers of this code.
You wont tell them :
"OK but dont worry we never call this function for real, why do you care
at all"
> >
> > 3) Now we use the xfrm_policy_afinfo_lock spinlock only from process
> > context, we no longer need to block BH in xfrm_policy_register_afinfo()
> > and xfrm_policy_unregister_afinfo()
> >
> I don't think it's related to what kinds of locks we are using.
> we call xfrm_policy_register_afinfo in process context, but actually
> what xfrm_policy_afinfo_lock protected can be used in soft irq context.
> that's why xx_bh is used in:
You did an RCU conversion and obviously have little idea of what
happened there.
This _bh stuff was needed because _before_ RCU, an rwlock was used.
And since read_lock() was used from BH handler, _all_ write_lock() had
to use the write_lock_bh() variant to avoid a possible deadlock.
But after RCU, this no longer is needed, as an rcu_read_lock() cannot
block a writer anymore in the lock/unlock section.
In fact, xfrm_policy_afinfo_lock could be replaced by a mutex. So _bh()
is absolutely not needed anymore.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] xfrm: fix RCU bugs
2012-08-20 5:33 ` Eric Dumazet
@ 2012-08-20 6:33 ` Fan Du
2012-08-20 7:14 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: Fan Du @ 2012-08-20 6:33 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, fengguang.wu, Priyanka Jain
On 2012年08月20日 13:33, Eric Dumazet wrote:
> On Mon, 2012-08-20 at 12:40 +0800, Fan Du wrote:
>> Hi Eric
>>
>> Please correct me if I'm wrong about below comments.
>>
>> On 2012年08月19日 18:31, Eric Dumazet wrote:
>>> From: Eric Dumazet<edumazet@google.com>
>>>
>>> This patch reverts commit 56892261ed1a (xfrm: Use rcu_dereference_bh to
>>> deference pointer protected by rcu_read_lock_bh), and fixes bugs
>>> introduced in commit 418a99ac6ad ( Replace rwlock on xfrm_policy_afinfo
>>> with rcu )
>>>
>>> 1) We properly use RCU variant in this file, not a mix of RCU/RCU_BH
>>>
>>> 2) We must defer some writes after the synchronize_rcu() call or a reader
>>> can crash dereferencing NULL pointer.
>>
>> Not exactly.
>>
>> net/ipv4/xfrm4_policy.c
>> static void __exit xfrm4_policy_fini(void)
>> -> xfrm_policy_unregister_afinfo
>>
>> IMHO, ip stack can never be compiled as module, so is xfrm4_policy_fini
>> freed up after system bootup? which means xfrm4_policy_fini can never be
>> called.
>>
>> so an dereferencing NULL pointer by a reader could not happen.
>>
>
> Last famous words.
>
> Anyway xfrm_policy_unregister_afinfo() is also called from
> xfrm6_policy_fini(), and IPv6 is a module. The day we can rmmod it,
> we uncover this bug.
>
> RCU is complex (most people dont get it right, thats the truth),
> and we should make it rock solid, or I can guarantee you
> many patch attempts from future readers of this code.
>
> You wont tell them :
>
> "OK but dont worry we never call this function for real, why do you care
> at all"
>
You are correct!
And one out of topic question:
The usage of xfrm_state_afinfo_lock/xfrm_km_lock is extremely
similar with xfrm_policy_afinfo_lock, except the former is not so
frequently read than that of the later.
Is it justified to convert RW xfrm_state_afinfo_lock/xfrm_km_lock into
RCU?
>>>
>>> 3) Now we use the xfrm_policy_afinfo_lock spinlock only from process
>>> context, we no longer need to block BH in xfrm_policy_register_afinfo()
>>> and xfrm_policy_unregister_afinfo()
>>>
>> I don't think it's related to what kinds of locks we are using.
>> we call xfrm_policy_register_afinfo in process context, but actually
>> what xfrm_policy_afinfo_lock protected can be used in soft irq context.
>> that's why xx_bh is used in:
>
> You did an RCU conversion and obviously have little idea of what
> happened there.
>
> This _bh stuff was needed because _before_ RCU, an rwlock was used.
>
> And since read_lock() was used from BH handler, _all_ write_lock() had
> to use the write_lock_bh() variant to avoid a possible deadlock.
>
> But after RCU, this no longer is needed, as an rcu_read_lock() cannot
> block a writer anymore in the lock/unlock section.
>
> In fact, xfrm_policy_afinfo_lock could be replaced by a mutex. So _bh()
> is absolutely not needed anymore.
>
I indeed misunderstood the code a bit.
Your explanation is crystal clear, thanks :)
>
>
--
Love each day!
--fan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] xfrm: fix RCU bugs
2012-08-20 6:33 ` Fan Du
@ 2012-08-20 7:14 ` Eric Dumazet
0 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2012-08-20 7:14 UTC (permalink / raw)
To: Fan Du; +Cc: David Miller, netdev, fengguang.wu, Priyanka Jain
On Mon, 2012-08-20 at 14:33 +0800, Fan Du wrote:
> And one out of topic question:
> The usage of xfrm_state_afinfo_lock/xfrm_km_lock is extremely
> similar with xfrm_policy_afinfo_lock, except the former is not so
> frequently read than that of the later.
>
> Is it justified to convert RW xfrm_state_afinfo_lock/xfrm_km_lock into
> RCU?
>
I would say it is justified, and easy enough.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] xfrm: fix RCU bugs
2012-08-19 10:31 [PATCH net-next] xfrm: fix RCU bugs Eric Dumazet
2012-08-20 4:40 ` Fan Du
@ 2012-08-23 5:40 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2012-08-23 5:40 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, fan.du, fengguang.wu, Priyanka.Jain
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 19 Aug 2012 12:31:48 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> This patch reverts commit 56892261ed1a (xfrm: Use rcu_dereference_bh to
> deference pointer protected by rcu_read_lock_bh), and fixes bugs
> introduced in commit 418a99ac6ad ( Replace rwlock on xfrm_policy_afinfo
> with rcu )
>
> 1) We properly use RCU variant in this file, not a mix of RCU/RCU_BH
>
> 2) We must defer some writes after the synchronize_rcu() call or a reader
> can crash dereferencing NULL pointer.
>
> 3) Now we use the xfrm_policy_afinfo_lock spinlock only from process
> context, we no longer need to block BH in xfrm_policy_register_afinfo()
> and xfrm_policy_unregister_afinfo()
>
> 4) Can use RCU_INIT_POINTER() instead of rcu_assign_pointer() in
> xfrm_policy_unregister_afinfo()
>
> 5) Remove a forward inline declaration (xfrm_policy_put_afinfo()),
> and also move xfrm_policy_get_afinfo() declaration.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-08-23 5:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-19 10:31 [PATCH net-next] xfrm: fix RCU bugs Eric Dumazet
2012-08-20 4:40 ` Fan Du
2012-08-20 5:33 ` Eric Dumazet
2012-08-20 6:33 ` Fan Du
2012-08-20 7:14 ` Eric Dumazet
2012-08-23 5:40 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).