diff mbox

[ipsec-next,6/8] xfrm: policy: only use rcu in xfrm_sk_policy_lookup

Message ID 20161116173028.GI30581@breakpoint.cc
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show

Commit Message

Florian Westphal Nov. 16, 2016, 5:30 p.m. UTC 
Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> Le 11/08/2016 à 15:17, Florian Westphal a écrit :
> > Don't acquire the readlock anymore and rely on rcu alone.
> > 
> > In case writer on other CPU changed policy at the wrong moment (after we
> > obtained sk policy pointer but before we could obtain the reference)
> > just repeat the lookup.
> > 
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> Since this patch, our IKEv1 Transport tests (using charon) fail to establish the
> connection. If I revert it, the IKE negociation is ok again.
> charon logs are enclosed.
> 
> I didn't had time to investigate now, but any idea is welcomed ;-)

I'm an idiot.  Thanks for figuring out the faulty commit!

This should fix it (if we succeed grabbing the refcount, then if (err && !xfrm_pol_hold_rcu
evaluates to false and we hit last else branch and set pol to ERR_PTR(0)...

Comments

Nicolas Dichtel Nov. 17, 2016, 8:55 a.m. UTC | #1 
Le 16/11/2016 à 18:30, Florian Westphal a écrit :
[snip]
> This should fix it (if we succeed grabbing the refcount, then if (err && !xfrm_pol_hold_rcu
> evaluates to false and we hit last else branch and set pol to ERR_PTR(0)...
Haha, right :) Thank you for the quick fix.
The patch solves my problem. You can add my 'tested-by' tag in the formal
submission.

Tested-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
diff mbox

Patch

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index fd6986634e6f..5bf7e1bfeac7 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1268,12 +1268,14 @@  static struct xfrm_policy *xfrm_sk_policy_lookup(const struct sock *sk, int dir,
                        err = security_xfrm_policy_lookup(pol->security,
                                                      fl->flowi_secid,
                                                      policy_to_flow_dir(dir));
-                       if (!err && !xfrm_pol_hold_rcu(pol))
-                               goto again;
-                       else if (err == -ESRCH)
+                       if (!err) {
+                               if (!xfrm_pol_hold_rcu(pol))
+                                       goto again;
+                       } else if (err == -ESRCH) {
                                pol = NULL;
-                       else
+                       } else {
                                pol = ERR_PTR(err);
+                       }
                } else