Skip to content
This repository has been archived by the owner on Apr 18, 2024. It is now read-only.

Commit

Permalink
mptcp: correct refcnt for sk in tcp_v{4,6}_rcv
Browse files Browse the repository at this point in the history
When we receive an skb in tcp_vX_rcv if the sock is hold by user we copy
the sk inside the skb before putting the skb inside the backlog, the sk
will be used after from mptcp_backlog_rcv. If the sk refcnt is not done
properly the sk read from mptcp_backlog_rcv could point to another sk
because it may have been freed and reused in the mean time.

In mptcp_backlog_rcv, the call to skb_oprhan will call the destructor
that we set for the skb and decrement the sk refcnt. Note that we call
skb_oprhan after the increase of sk_refcnt. This ensures that we never
free the sk too quickly.

We also remove unneeded skb->sk assignments, a line is there for that in
mptcp_backlog_rcv and if we set it, we should also assign a destructor.

Notes about backports/forwardport:
- in mptcp_trunk, changes in mptcp_input.c should no longer be needed.
  In tcp_ipv{4,6}, the first additions should no longer be needed as
  well.
- before v4.4, we need to remove skb->sk = meta_sk in mptcp_check_req().

Fixes: Zero-day-bug
Signed-off-by: Gregory Detal <gregory.detal@tessares.net>
Signed-off-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Christoph Paasch <cpaasch@apple.com>
  • Loading branch information
gdetal authored and cpaasch committed Aug 14, 2018
1 parent 80b661f commit 952bd28
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 8 deletions.
2 changes: 2 additions & 0 deletions include/net/mptcp.h
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,7 @@ void mptcp_sub_close_wq(struct work_struct *work);
void mptcp_sub_close(struct sock *sk, unsigned long delay);
struct sock *mptcp_select_ack_sock(const struct sock *meta_sk);
void mptcp_fallback_meta_sk(struct sock *meta_sk);
void mptcp_prepare_for_backlog(struct sock *sk, struct sk_buff *skb);
int mptcp_backlog_rcv(struct sock *meta_sk, struct sk_buff *skb);
void mptcp_ack_handler(unsigned long);
bool mptcp_check_rtt(const struct tcp_sock *tp, int time);
Expand Down Expand Up @@ -1438,6 +1439,7 @@ static inline bool mptcp_fallback_infinite(const struct sock *sk, int flag)
return false;
}
static inline void mptcp_init_mp_opt(const struct mptcp_options_received *mopt) {}
static inline void mptcp_prepare_for_backlog(struct sock *sk, struct sk_buff *skb) {}
static inline bool mptcp_check_rtt(const struct tcp_sock *tp, int time)
{
return false;
Expand Down
4 changes: 2 additions & 2 deletions net/ipv4/tcp_ipv4.c
Original file line number Diff line number Diff line change
Expand Up @@ -1742,7 +1742,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
}

if (sock_owned_by_user(sk)) {
skb->sk = sk;
mptcp_prepare_for_backlog(sk, skb);
if (unlikely(sk_add_backlog(sk, skb,
sk->sk_rcvbuf + sk->sk_sndbuf))) {
reqsk_put(req);
Expand Down Expand Up @@ -1819,7 +1819,7 @@ int tcp_v4_rcv(struct sk_buff *skb)

bh_lock_sock_nested(meta_sk);
if (sock_owned_by_user(meta_sk))
skb->sk = sk;
mptcp_prepare_for_backlog(sk, skb);
} else {
meta_sk = sk;
bh_lock_sock_nested(sk);
Expand Down
4 changes: 2 additions & 2 deletions net/ipv6/tcp_ipv6.c
Original file line number Diff line number Diff line change
Expand Up @@ -1530,7 +1530,7 @@ static int tcp_v6_rcv(struct sk_buff *skb)
}

if (sock_owned_by_user(sk)) {
skb->sk = sk;
mptcp_prepare_for_backlog(sk, skb);
if (unlikely(sk_add_backlog(sk, skb,
sk->sk_rcvbuf + sk->sk_sndbuf))) {
reqsk_put(req);
Expand Down Expand Up @@ -1606,7 +1606,7 @@ static int tcp_v6_rcv(struct sk_buff *skb)

bh_lock_sock_nested(meta_sk);
if (sock_owned_by_user(meta_sk))
skb->sk = sk;
mptcp_prepare_for_backlog(sk, skb);
} else {
meta_sk = sk;
bh_lock_sock_nested(sk);
Expand Down
18 changes: 16 additions & 2 deletions net/mptcp/mptcp_ctrl.c
Original file line number Diff line number Diff line change
Expand Up @@ -984,6 +984,16 @@ static void mptcp_sub_inherit_sockopts(const struct sock *meta_sk, struct sock *
inet_sk(sub_sk)->recverr = 0;
}

void mptcp_prepare_for_backlog(struct sock *sk, struct sk_buff *skb)
{
/* In case of success (in mptcp_backlog_rcv) and error (in kfree_skb) of
* sk_add_backlog, we will decrement the sk refcount.
*/
sock_hold(sk);
skb->sk = sk;
skb->destructor = sock_efree;
}

int mptcp_backlog_rcv(struct sock *meta_sk, struct sk_buff *skb)
{
/* skb-sk may be NULL if we receive a packet immediatly after the
Expand All @@ -992,13 +1002,17 @@ int mptcp_backlog_rcv(struct sock *meta_sk, struct sk_buff *skb)
struct sock *sk = skb->sk ? skb->sk : meta_sk;
int ret = 0;

skb->sk = NULL;

if (unlikely(!refcount_inc_not_zero(&sk->sk_refcnt))) {
kfree_skb(skb);
return 0;
}

/* Decrement sk refcnt when calling the skb destructor.
* Refcnt is incremented and skb destructor is set in tcp_v{4,6}_rcv via
* mptcp_prepare_for_backlog() here above.
*/
skb_orphan(skb);

if (sk->sk_family == AF_INET)
ret = tcp_v4_do_rcv(sk, skb);
#if IS_ENABLED(CONFIG_IPV6)
Expand Down
2 changes: 0 additions & 2 deletions net/mptcp/mptcp_input.c
Original file line number Diff line number Diff line change
Expand Up @@ -1184,7 +1184,6 @@ int mptcp_lookup_join(struct sk_buff *skb, struct inet_timewait_sock *tw)
*/
bh_lock_sock_nested(meta_sk);
if (sock_owned_by_user(meta_sk)) {
skb->sk = meta_sk;
if (unlikely(sk_add_backlog(meta_sk, skb,
meta_sk->sk_rcvbuf + meta_sk->sk_sndbuf))) {
bh_unlock_sock(meta_sk);
Expand Down Expand Up @@ -1257,7 +1256,6 @@ int mptcp_do_join_short(struct sk_buff *skb,
}

if (sock_owned_by_user(meta_sk)) {
skb->sk = meta_sk;
if (unlikely(sk_add_backlog(meta_sk, skb,
meta_sk->sk_rcvbuf + meta_sk->sk_sndbuf)))
__NET_INC_STATS(net, LINUX_MIB_TCPBACKLOGDROP);
Expand Down

0 comments on commit 952bd28

Please sign in to comment.