lkml.org 
[lkml]   [2015]   [Oct]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 3.2 027/107] net: Fix RCU splat in af_key
    3.2.72-rc1 review patch.  If anyone has any objections, please let me know.

    ------------------

    From: David Ahern <dsa@cumulusnetworks.com>

    commit ba51b6be38c122f7dab40965b4397aaf6188a464 upstream.

    Hit the following splat testing VRF change for ipsec:

    [ 113.475692] ===============================
    [ 113.476194] [ INFO: suspicious RCU usage. ]
    [ 113.476667] 4.2.0-rc6-1+deb7u2+clUNRELEASED #3.2.65-1+deb7u2+clUNRELEASED Not tainted
    [ 113.477545] -------------------------------
    [ 113.478013] /work/monster-14/dsa/kernel.git/include/linux/rcupdate.h:568 Illegal context switch in RCU read-side critical section!
    [ 113.479288]
    [ 113.479288] other info that might help us debug this:
    [ 113.479288]
    [ 113.480207]
    [ 113.480207] rcu_scheduler_active = 1, debug_locks = 1
    [ 113.480931] 2 locks held by setkey/6829:
    [ 113.481371] #0: (&net->xfrm.xfrm_cfg_mutex){+.+.+.}, at: [<ffffffff814e9887>] pfkey_sendmsg+0xfb/0x213
    [ 113.482509] #1: (rcu_read_lock){......}, at: [<ffffffff814e767f>] rcu_read_lock+0x0/0x6e
    [ 113.483509]
    [ 113.483509] stack backtrace:
    [ 113.484041] CPU: 0 PID: 6829 Comm: setkey Not tainted 4.2.0-rc6-1+deb7u2+clUNRELEASED #3.2.65-1+deb7u2+clUNRELEASED
    [ 113.485422] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.7.5.1-0-g8936dbb-20141113_115728-nilsson.home.kraxel.org 04/01/2014
    [ 113.486845] 0000000000000001 ffff88001d4c7a98 ffffffff81518af2 ffffffff81086962
    [ 113.487732] ffff88001d538480 ffff88001d4c7ac8 ffffffff8107ae75 ffffffff8180a154
    [ 113.488628] 0000000000000b30 0000000000000000 00000000000000d0 ffff88001d4c7ad8
    [ 113.489525] Call Trace:
    [ 113.489813] [<ffffffff81518af2>] dump_stack+0x4c/0x65
    [ 113.490389] [<ffffffff81086962>] ? console_unlock+0x3d6/0x405
    [ 113.491039] [<ffffffff8107ae75>] lockdep_rcu_suspicious+0xfa/0x103
    [ 113.491735] [<ffffffff81064032>] rcu_preempt_sleep_check+0x45/0x47
    [ 113.492442] [<ffffffff8106404d>] ___might_sleep+0x19/0x1c8
    [ 113.493077] [<ffffffff81064268>] __might_sleep+0x6c/0x82
    [ 113.493681] [<ffffffff81133190>] cache_alloc_debugcheck_before.isra.50+0x1d/0x24
    [ 113.494508] [<ffffffff81134876>] kmem_cache_alloc+0x31/0x18f
    [ 113.495149] [<ffffffff814012b5>] skb_clone+0x64/0x80
    [ 113.495712] [<ffffffff814e6f71>] pfkey_broadcast_one+0x3d/0xff
    [ 113.496380] [<ffffffff814e7b84>] pfkey_broadcast+0xb5/0x11e
    [ 113.497024] [<ffffffff814e82d1>] pfkey_register+0x191/0x1b1
    [ 113.497653] [<ffffffff814e9770>] pfkey_process+0x162/0x17e
    [ 113.498274] [<ffffffff814e9895>] pfkey_sendmsg+0x109/0x213

    In pfkey_sendmsg the net mutex is taken and then pfkey_broadcast takes
    the RCU lock.

    Since pfkey_broadcast takes the RCU lock the allocation argument is
    pointless since GFP_ATOMIC must be used between the rcu_read_{,un}lock.
    The one call outside of rcu can be done with GFP_KERNEL.

    Fixes: 7f6b9dbd5afbd ("af_key: locking change")
    Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
    Acked-by: Eric Dumazet <edumazet@google.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    [bwh: Backported to 3.2: adjust context]
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    net/key/af_key.c | 46 +++++++++++++++++++++++-----------------------
    1 file changed, 23 insertions(+), 23 deletions(-)

    --- a/net/key/af_key.c
    +++ b/net/key/af_key.c
    @@ -220,7 +220,7 @@ static int pfkey_broadcast_one(struct sk
    #define BROADCAST_ONE 1
    #define BROADCAST_REGISTERED 2
    #define BROADCAST_PROMISC_ONLY 4
    -static int pfkey_broadcast(struct sk_buff *skb, gfp_t allocation,
    +static int pfkey_broadcast(struct sk_buff *skb,
    int broadcast_flags, struct sock *one_sk,
    struct net *net)
    {
    @@ -246,7 +246,7 @@ static int pfkey_broadcast(struct sk_buf
    * socket.
    */
    if (pfk->promisc)
    - pfkey_broadcast_one(skb, &skb2, allocation, sk);
    + pfkey_broadcast_one(skb, &skb2, GFP_ATOMIC, sk);

    /* the exact target will be processed later */
    if (sk == one_sk)
    @@ -261,7 +261,7 @@ static int pfkey_broadcast(struct sk_buf
    continue;
    }

    - err2 = pfkey_broadcast_one(skb, &skb2, allocation, sk);
    + err2 = pfkey_broadcast_one(skb, &skb2, GFP_ATOMIC, sk);

    /* Error is cleare after succecful sending to at least one
    * registered KM */
    @@ -271,7 +271,7 @@ static int pfkey_broadcast(struct sk_buf
    rcu_read_unlock();

    if (one_sk != NULL)
    - err = pfkey_broadcast_one(skb, &skb2, allocation, one_sk);
    + err = pfkey_broadcast_one(skb, &skb2, GFP_KERNEL, one_sk);

    kfree_skb(skb2);
    kfree_skb(skb);
    @@ -294,7 +294,7 @@ static int pfkey_do_dump(struct pfkey_so
    hdr = (struct sadb_msg *) pfk->dump.skb->data;
    hdr->sadb_msg_seq = 0;
    hdr->sadb_msg_errno = rc;
    - pfkey_broadcast(pfk->dump.skb, GFP_ATOMIC, BROADCAST_ONE,
    + pfkey_broadcast(pfk->dump.skb, BROADCAST_ONE,
    &pfk->sk, sock_net(&pfk->sk));
    pfk->dump.skb = NULL;
    }
    @@ -335,7 +335,7 @@ static int pfkey_error(const struct sadb
    hdr->sadb_msg_len = (sizeof(struct sadb_msg) /
    sizeof(uint64_t));

    - pfkey_broadcast(skb, GFP_KERNEL, BROADCAST_ONE, sk, sock_net(sk));
    + pfkey_broadcast(skb, BROADCAST_ONE, sk, sock_net(sk));

    return 0;
    }
    @@ -1361,7 +1361,7 @@ static int pfkey_getspi(struct sock *sk,

    xfrm_state_put(x);

    - pfkey_broadcast(resp_skb, GFP_KERNEL, BROADCAST_ONE, sk, net);
    + pfkey_broadcast(resp_skb, BROADCAST_ONE, sk, net);

    return 0;
    }
    @@ -1449,7 +1449,7 @@ static int key_notify_sa(struct xfrm_sta
    hdr->sadb_msg_seq = c->seq;
    hdr->sadb_msg_pid = c->pid;

    - pfkey_broadcast(skb, GFP_ATOMIC, BROADCAST_ALL, NULL, xs_net(x));
    + pfkey_broadcast(skb, BROADCAST_ALL, NULL, xs_net(x));

    return 0;
    }
    @@ -1566,7 +1566,7 @@ static int pfkey_get(struct sock *sk, st
    out_hdr->sadb_msg_reserved = 0;
    out_hdr->sadb_msg_seq = hdr->sadb_msg_seq;
    out_hdr->sadb_msg_pid = hdr->sadb_msg_pid;
    - pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ONE, sk, sock_net(sk));
    + pfkey_broadcast(out_skb, BROADCAST_ONE, sk, sock_net(sk));

    return 0;
    }
    @@ -1667,7 +1667,7 @@ static int pfkey_register(struct sock *s
    return -ENOBUFS;
    }

    - pfkey_broadcast(supp_skb, GFP_KERNEL, BROADCAST_REGISTERED, sk, sock_net(sk));
    + pfkey_broadcast(supp_skb, BROADCAST_REGISTERED, sk, sock_net(sk));

    return 0;
    }
    @@ -1686,7 +1686,7 @@ static int unicast_flush_resp(struct soc
    hdr->sadb_msg_errno = (uint8_t) 0;
    hdr->sadb_msg_len = (sizeof(struct sadb_msg) / sizeof(uint64_t));

    - return pfkey_broadcast(skb, GFP_ATOMIC, BROADCAST_ONE, sk, sock_net(sk));
    + return pfkey_broadcast(skb, BROADCAST_ONE, sk, sock_net(sk));
    }

    static int key_notify_sa_flush(const struct km_event *c)
    @@ -1707,7 +1707,7 @@ static int key_notify_sa_flush(const str
    hdr->sadb_msg_len = (sizeof(struct sadb_msg) / sizeof(uint64_t));
    hdr->sadb_msg_reserved = 0;

    - pfkey_broadcast(skb, GFP_ATOMIC, BROADCAST_ALL, NULL, c->net);
    + pfkey_broadcast(skb, BROADCAST_ALL, NULL, c->net);

    return 0;
    }
    @@ -1768,7 +1768,7 @@ static int dump_sa(struct xfrm_state *x,
    out_hdr->sadb_msg_pid = pfk->dump.msg_pid;

    if (pfk->dump.skb)
    - pfkey_broadcast(pfk->dump.skb, GFP_ATOMIC, BROADCAST_ONE,
    + pfkey_broadcast(pfk->dump.skb, BROADCAST_ONE,
    &pfk->sk, sock_net(&pfk->sk));
    pfk->dump.skb = out_skb;

    @@ -1829,7 +1829,7 @@ static int pfkey_promisc(struct sock *sk
    new_hdr->sadb_msg_errno = 0;
    }

    - pfkey_broadcast(skb, GFP_KERNEL, BROADCAST_ALL, NULL, sock_net(sk));
    + pfkey_broadcast(skb, BROADCAST_ALL, NULL, sock_net(sk));
    return 0;
    }

    @@ -2160,7 +2160,7 @@ static int key_notify_policy(struct xfrm
    out_hdr->sadb_msg_errno = 0;
    out_hdr->sadb_msg_seq = c->seq;
    out_hdr->sadb_msg_pid = c->pid;
    - pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ALL, NULL, xp_net(xp));
    + pfkey_broadcast(out_skb, BROADCAST_ALL, NULL, xp_net(xp));
    return 0;

    }
    @@ -2386,7 +2386,7 @@ static int key_pol_get_resp(struct sock
    out_hdr->sadb_msg_errno = 0;
    out_hdr->sadb_msg_seq = hdr->sadb_msg_seq;
    out_hdr->sadb_msg_pid = hdr->sadb_msg_pid;
    - pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ONE, sk, xp_net(xp));
    + pfkey_broadcast(out_skb, BROADCAST_ONE, sk, xp_net(xp));
    err = 0;

    out:
    @@ -2639,7 +2639,7 @@ static int dump_sp(struct xfrm_policy *x
    out_hdr->sadb_msg_pid = pfk->dump.msg_pid;

    if (pfk->dump.skb)
    - pfkey_broadcast(pfk->dump.skb, GFP_ATOMIC, BROADCAST_ONE,
    + pfkey_broadcast(pfk->dump.skb, BROADCAST_ONE,
    &pfk->sk, sock_net(&pfk->sk));
    pfk->dump.skb = out_skb;

    @@ -2690,7 +2690,7 @@ static int key_notify_policy_flush(const
    hdr->sadb_msg_satype = SADB_SATYPE_UNSPEC;
    hdr->sadb_msg_len = (sizeof(struct sadb_msg) / sizeof(uint64_t));
    hdr->sadb_msg_reserved = 0;
    - pfkey_broadcast(skb_out, GFP_ATOMIC, BROADCAST_ALL, NULL, c->net);
    + pfkey_broadcast(skb_out, BROADCAST_ALL, NULL, c->net);
    return 0;

    }
    @@ -2756,7 +2756,7 @@ static int pfkey_process(struct sock *sk
    void *ext_hdrs[SADB_EXT_MAX];
    int err;

    - pfkey_broadcast(skb_clone(skb, GFP_KERNEL), GFP_KERNEL,
    + pfkey_broadcast(skb_clone(skb, GFP_KERNEL),
    BROADCAST_PROMISC_ONLY, NULL, sock_net(sk));

    memset(ext_hdrs, 0, sizeof(ext_hdrs));
    @@ -2962,7 +2962,7 @@ static int key_notify_sa_expire(struct x
    out_hdr->sadb_msg_seq = 0;
    out_hdr->sadb_msg_pid = 0;

    - pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_REGISTERED, NULL, xs_net(x));
    + pfkey_broadcast(out_skb, BROADCAST_REGISTERED, NULL, xs_net(x));
    return 0;
    }

    @@ -3134,7 +3134,7 @@ static int pfkey_send_acquire(struct xfr
    xfrm_ctx->ctx_len);
    }

    - return pfkey_broadcast(skb, GFP_ATOMIC, BROADCAST_REGISTERED, NULL, xs_net(x));
    + return pfkey_broadcast(skb, BROADCAST_REGISTERED, NULL, xs_net(x));
    }

    static struct xfrm_policy *pfkey_compile_policy(struct sock *sk, int opt,
    @@ -3332,7 +3332,7 @@ static int pfkey_send_new_mapping(struct
    n_port->sadb_x_nat_t_port_port = sport;
    n_port->sadb_x_nat_t_port_reserved = 0;

    - return pfkey_broadcast(skb, GFP_ATOMIC, BROADCAST_REGISTERED, NULL, xs_net(x));
    + return pfkey_broadcast(skb, BROADCAST_REGISTERED, NULL, xs_net(x));
    }

    #ifdef CONFIG_NET_KEY_MIGRATE
    @@ -3524,7 +3524,7 @@ static int pfkey_send_migrate(const stru
    }

    /* broadcast migrate message to sockets */
    - pfkey_broadcast(skb, GFP_ATOMIC, BROADCAST_ALL, NULL, &init_net);
    + pfkey_broadcast(skb, BROADCAST_ALL, NULL, &init_net);

    return 0;



    \
     
     \ /
      Last update: 2015-10-09 02:41    [W:3.281 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site