lkml.org 
[lkml]   [2017]   [Nov]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 3.16 082/133] l2tp: pass tunnel pointer to ->session_create()
    3.16.51-rc1 review patch.  If anyone has any objections, please let me know.

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

    From: Guillaume Nault <g.nault@alphalink.fr>

    commit f026bc29a8e093edfbb2a77700454b285c97e8ad upstream.

    Using l2tp_tunnel_find() in pppol2tp_session_create() and
    l2tp_eth_create() is racy, because no reference is held on the
    returned session. These functions are only used to implement the
    ->session_create callback which is run by l2tp_nl_cmd_session_create().
    Therefore searching for the parent tunnel isn't necessary because
    l2tp_nl_cmd_session_create() already has a pointer to it and holds a
    reference.

    This patch modifies ->session_create()'s prototype to directly pass the
    the parent tunnel as parameter, thus avoiding searching for it in
    pppol2tp_session_create() and l2tp_eth_create().

    Since we have to touch the ->session_create() call in
    l2tp_nl_cmd_session_create(), let's also remove the useless conditional:
    we know that ->session_create isn't NULL at this point because it's
    already been checked earlier in this same function.

    Finally, one might be tempted to think that the removed
    l2tp_tunnel_find() calls were harmless because they would return the
    same tunnel as the one held by l2tp_nl_cmd_session_create() anyway.
    But that tunnel might be removed and a new one created with same tunnel
    Id before the l2tp_tunnel_find() call. In this case l2tp_tunnel_find()
    would return the new tunnel which wouldn't be protected by the
    reference held by l2tp_nl_cmd_session_create().

    Fixes: 309795f4bec2 ("l2tp: Add netlink control API for L2TP")
    Fixes: d9e31d17ceba ("l2tp: Add L2TP ethernet pseudowire support")
    Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    [bwh: Backported to 3.16: adjust context]
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    net/l2tp/l2tp_core.h | 4 +++-
    net/l2tp/l2tp_eth.c | 11 +++--------
    net/l2tp/l2tp_netlink.c | 8 ++++----
    net/l2tp/l2tp_ppp.c | 19 +++++++------------
    4 files changed, 17 insertions(+), 25 deletions(-)

    --- a/net/l2tp/l2tp_core.h
    +++ b/net/l2tp/l2tp_core.h
    @@ -210,7 +210,9 @@ struct l2tp_tunnel {
    };

    struct l2tp_nl_cmd_ops {
    - int (*session_create)(struct net *net, u32 tunnel_id, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg);
    + int (*session_create)(struct net *net, struct l2tp_tunnel *tunnel,
    + u32 session_id, u32 peer_session_id,
    + struct l2tp_session_cfg *cfg);
    int (*session_delete)(struct l2tp_session *session);
    };

    --- a/net/l2tp/l2tp_eth.c
    +++ b/net/l2tp/l2tp_eth.c
    @@ -206,23 +206,18 @@ static void l2tp_eth_show(struct seq_fil
    }
    #endif

    -static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg)
    +static int l2tp_eth_create(struct net *net, struct l2tp_tunnel *tunnel,
    + u32 session_id, u32 peer_session_id,
    + struct l2tp_session_cfg *cfg)
    {
    struct net_device *dev;
    char name[IFNAMSIZ];
    - struct l2tp_tunnel *tunnel;
    struct l2tp_session *session;
    struct l2tp_eth *priv;
    struct l2tp_eth_sess *spriv;
    int rc;
    struct l2tp_eth_net *pn;

    - tunnel = l2tp_tunnel_find(net, tunnel_id);
    - if (!tunnel) {
    - rc = -ENODEV;
    - goto out;
    - }
    -
    if (cfg->ifname) {
    dev = dev_get_by_name(net, cfg->ifname);
    if (dev) {
    --- a/net/l2tp/l2tp_netlink.c
    +++ b/net/l2tp/l2tp_netlink.c
    @@ -540,10 +540,10 @@ static int l2tp_nl_cmd_session_create(st
    break;
    }

    - ret = -EPROTONOSUPPORT;
    - if (l2tp_nl_cmd_ops[cfg.pw_type]->session_create)
    - ret = (*l2tp_nl_cmd_ops[cfg.pw_type]->session_create)(net, tunnel_id,
    - session_id, peer_session_id, &cfg);
    + ret = l2tp_nl_cmd_ops[cfg.pw_type]->session_create(net, tunnel,
    + session_id,
    + peer_session_id,
    + &cfg);

    out_tunnel:
    l2tp_tunnel_dec_refcount(tunnel);
    --- a/net/l2tp/l2tp_ppp.c
    +++ b/net/l2tp/l2tp_ppp.c
    @@ -807,25 +807,20 @@ end:

    #ifdef CONFIG_L2TP_V3

    -/* Called when creating sessions via the netlink interface.
    - */
    -static int pppol2tp_session_create(struct net *net, u32 tunnel_id, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg)
    +/* Called when creating sessions via the netlink interface. */
    +static int pppol2tp_session_create(struct net *net, struct l2tp_tunnel *tunnel,
    + u32 session_id, u32 peer_session_id,
    + struct l2tp_session_cfg *cfg)
    {
    int error;
    - struct l2tp_tunnel *tunnel;
    struct l2tp_session *session;
    struct pppol2tp_session *ps;

    - tunnel = l2tp_tunnel_find(net, tunnel_id);
    -
    - /* Error if we can't find the tunnel */
    - error = -ENOENT;
    - if (tunnel == NULL)
    - goto out;
    -
    /* Error if tunnel socket is not prepped */
    - if (tunnel->sock == NULL)
    + if (!tunnel->sock) {
    + error = -ENOENT;
    goto out;
    + }

    /* Default MTU values. */
    if (cfg->mtu == 0)
    \
     
     \ /
      Last update: 2017-11-22 03:31    [W:6.945 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site