lkml.org 
[lkml]   [2018]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 4.4 063/114] l2tp: hold socket before dropping lock in l2tp_ip{, 6}_recv()
    On Thu, Nov 08, 2018 at 01:51:18PM -0800, Greg Kroah-Hartman wrote:
    > 4.4-stable review patch. If anyone has any objections, please let me know.
    >
    This patch was partly wrong. Here is the followup fix:
    94d7ee0baa8b ("l2tp: hold tunnel socket when handling control frames in l2tp_ip and l2tp_ip6")

    Both a3c18422a4 ("l2tp: hold socket before dropping lock in l2tp_ip{, 6}_recv()")
    and 94d7ee0baa8b ("l2tp: hold tunnel socket when handling control frames in l2tp_ip and l2tp_ip6")
    should be applied/backported together.

    > ------------------
    >
    > [ Upstream commit a3c18422a4b4e108bcf6a2328f48867e1003fd95 ]
    >
    > Socket must be held while under the protection of the l2tp lock; there
    > is no guarantee that sk remains valid after the read_unlock_bh() call.
    >
    > Same issue for l2tp_ip and l2tp_ip6.
    >
    > Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
    > Signed-off-by: David S. Miller <davem@davemloft.net>
    > Signed-off-by: Sasha Levin <sashal@kernel.org>
    > ---
    > net/l2tp/l2tp_ip.c | 11 ++++++-----
    > net/l2tp/l2tp_ip6.c | 11 ++++++-----
    > 2 files changed, 12 insertions(+), 10 deletions(-)
    >
    > diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c
    > index 48ab93842322..c7e6098c924e 100644
    > --- a/net/l2tp/l2tp_ip.c
    > +++ b/net/l2tp/l2tp_ip.c
    > @@ -184,14 +184,15 @@ pass_up:
    >
    > read_lock_bh(&l2tp_ip_lock);
    > sk = __l2tp_ip_bind_lookup(net, iph->daddr, 0, tunnel_id);
    > + if (!sk) {
    > + read_unlock_bh(&l2tp_ip_lock);
    > + goto discard;
    > + }
    > +
    > + sock_hold(sk);
    sock_hold(sk) is missing in the 'if' branch of this condition.

    > read_unlock_bh(&l2tp_ip_lock);
    > }
    >
    > - if (sk == NULL)
    > - goto discard;
    > -
    > - sock_hold(sk);
    > -
    Original sock_hold(sk) covered both parts of the condition.

    > if (!xfrm4_policy_check(sk, XFRM_POLICY_IN, skb))
    > goto discard_put;
    >
    > diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
    > index bcdab1cba773..5fe0a6f6af3d 100644
    > --- a/net/l2tp/l2tp_ip6.c
    > +++ b/net/l2tp/l2tp_ip6.c
    > @@ -196,14 +196,15 @@ pass_up:
    > read_lock_bh(&l2tp_ip6_lock);
    > sk = __l2tp_ip6_bind_lookup(&init_net, &iph->daddr,
    > 0, tunnel_id);
    > + if (!sk) {
    > + read_unlock_bh(&l2tp_ip6_lock);
    > + goto discard;
    > + }
    > +
    > + sock_hold(sk);
    Same here.

    > read_unlock_bh(&l2tp_ip6_lock);
    > }
    >
    > - if (sk == NULL)
    > - goto discard;
    > -
    > - sock_hold(sk);
    > -
    > if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
    > goto discard_put;
    >
    > --
    > 2.17.1
    >

    \
     
     \ /
      Last update: 2018-11-09 16:35    [W:4.093 / U:0.232 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site