author | Hanjie Lin <hanjie.lin@amlogic.com> | 2019-08-14 08:36:56 (GMT) |
---|---|---|
committer | Jianxin Pan <jianxin.pan@amlogic.com> | 2019-09-18 06:18:19 (GMT) |
commit | 64bdf85493769b57ee705648f6d26afd703ed0dc (patch) | |
tree | bc6d5d36c1e4fcb38f6f3dae133c7c46223829f5 | |
parent | 8663100e9d5986889b7d5939832ba03d21e9667d (diff) | |
download | common-64bdf85493769b57ee705648f6d26afd703ed0dc.zip common-64bdf85493769b57ee705648f6d26afd703ed0dc.tar.gz common-64bdf85493769b57ee705648f6d26afd703ed0dc.tar.bz2 |
RAVENPLAT 2378:OSS vulnerability found in [boot.img]:[linux_kernel] (CVE-2018-9517) Risk:[] [1/1]
PD#OTT-5666
[Problem]
In pppol2tp_connect, there is possible memory corruption due to a
use after free. This could lead to local escalation of privilege with
System execution privileges needed. User interaction is not needed for
exploitation.
[Solution]
l2tp: pass tunnel pointer to ->session_create()
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().
Change-Id: I50e19ae5abb4009205e59105222bf92e3587f9c4
Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
-rw-r--r-- | net/l2tp/l2tp_core.h | 4 | ||||
-rw-r--r-- | net/l2tp/l2tp_eth.c | 11 | ||||
-rw-r--r-- | net/l2tp/l2tp_netlink.c | 8 | ||||
-rw-r--r-- | net/l2tp/l2tp_ppp.c | 19 |
4 files changed, 17 insertions, 25 deletions
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 42419f1..13d9a42 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -209,7 +209,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); }; diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c index eecc64e..e30855d 100644 --- a/net/l2tp/l2tp_eth.c +++ b/net/l2tp/l2tp_eth.c @@ -206,23 +206,18 @@ static void l2tp_eth_show(struct seq_file *m, void *arg) } #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) { diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c index ee03bc8..18b5dfd 100644 --- a/net/l2tp/l2tp_netlink.c +++ b/net/l2tp/l2tp_netlink.c @@ -630,10 +630,10 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf 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); if (ret >= 0) { session = l2tp_session_get(net, tunnel, session_id, false); diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index 9b214f3..4a7b1e0 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -795,25 +795,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) |