Commit 72e15bb2 authored by Hannes Frederic Sowa's avatar Hannes Frederic Sowa Committed by Luis Henriques

pptp: fix illegal memory access caused by multiple bind()s

commit 9a368aff upstream.

Several times already this has been reported as kasan reports caused by
syzkaller and trinity and people always looked at RCU races, but it is
much more simple. :)

In case we bind a pptp socket multiple times, we simply add it to
the callid_sock list but don't remove the old binding. Thus the old
socket stays in the bucket with unused call_id indexes and doesn't get
cleaned up. This causes various forms of kasan reports which were hard
to pinpoint.

Simply don't allow multiple binds and correct error handling in
pptp_bind. Also keep sk_state bits in place in pptp_connect.

Fixes: 00959ade ("PPTP: PPP over IPv4 (Point-to-Point Tunneling Protocol)")
Cc: Dmitry Kozlov <xeb@mail.ru>
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Reported-by: default avatarDmitry Vyukov <dvyukov@google.com>
Cc: Dave Jones <davej@codemonkey.org.uk>
Reported-by: default avatarDave Jones <davej@codemonkey.org.uk>
Signed-off-by: default avatarHannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
Signed-off-by: default avatarLuis Henriques <luis.henriques@canonical.com>
parent 1906035d
......@@ -131,24 +131,27 @@ static int lookup_chan_dst(u16 call_id, __be32 d_addr)
return i < MAX_CALLID;
}
static int add_chan(struct pppox_sock *sock)
static int add_chan(struct pppox_sock *sock,
struct pptp_addr *sa)
{
static int call_id;
spin_lock(&chan_lock);
if (!sock->proto.pptp.src_addr.call_id) {
if (!sa->call_id) {
call_id = find_next_zero_bit(callid_bitmap, MAX_CALLID, call_id + 1);
if (call_id == MAX_CALLID) {
call_id = find_next_zero_bit(callid_bitmap, MAX_CALLID, 1);
if (call_id == MAX_CALLID)
goto out_err;
}
sock->proto.pptp.src_addr.call_id = call_id;
} else if (test_bit(sock->proto.pptp.src_addr.call_id, callid_bitmap))
sa->call_id = call_id;
} else if (test_bit(sa->call_id, callid_bitmap)) {
goto out_err;
}
set_bit(sock->proto.pptp.src_addr.call_id, callid_bitmap);
rcu_assign_pointer(callid_sock[sock->proto.pptp.src_addr.call_id], sock);
sock->proto.pptp.src_addr = *sa;
set_bit(sa->call_id, callid_bitmap);
rcu_assign_pointer(callid_sock[sa->call_id], sock);
spin_unlock(&chan_lock);
return 0;
......@@ -417,7 +420,6 @@ static int pptp_bind(struct socket *sock, struct sockaddr *uservaddr,
struct sock *sk = sock->sk;
struct sockaddr_pppox *sp = (struct sockaddr_pppox *) uservaddr;
struct pppox_sock *po = pppox_sk(sk);
struct pptp_opt *opt = &po->proto.pptp;
int error = 0;
if (sockaddr_len < sizeof(struct sockaddr_pppox))
......@@ -425,10 +427,22 @@ static int pptp_bind(struct socket *sock, struct sockaddr *uservaddr,
lock_sock(sk);
opt->src_addr = sp->sa_addr.pptp;
if (add_chan(po))
if (sk->sk_state & PPPOX_DEAD) {
error = -EALREADY;
goto out;
}
if (sk->sk_state & PPPOX_BOUND) {
error = -EBUSY;
goto out;
}
if (add_chan(po, &sp->sa_addr.pptp))
error = -EBUSY;
else
sk->sk_state |= PPPOX_BOUND;
out:
release_sock(sk);
return error;
}
......@@ -499,7 +513,7 @@ static int pptp_connect(struct socket *sock, struct sockaddr *uservaddr,
}
opt->dst_addr = sp->sa_addr.pptp;
sk->sk_state = PPPOX_CONNECTED;
sk->sk_state |= PPPOX_CONNECTED;
end:
release_sock(sk);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment