inet: fix possible panic in reqsk_queue_unlink()
[ 3897.923145] BUG: unable to handle kernel NULL pointer dereference at 0000000000000080 [ 3897.931025] IP: [<ffffffffa9f27686>] reqsk_timer_handler+0x1a6/0x243 There is a race when reqsk_timer_handler() and tcp_check_req() call inet_csk_reqsk_queue_unlink() on the same req at the same time. Before commitfa76ce7328
("inet: get rid of central tcp/dccp listener timer"), listener spinlock was held and race could not happen. To solve this bug, we change reqsk_queue_unlink() to not assume req must be found, and we return a status, to conditionally release a refcount on the request sock. This also means tcp_check_req() in non fastopen case might or not consume req refcount, so tcp_v6_hnd_req() & tcp_v4_hnd_req() have to properly handle this. (Same remark for dccp_check_req() and its callers) inet_csk_reqsk_queue_drop() is now too big to be inlined, as it is called 4 times in tcp and 3 times in dccp. Fixes:fa76ce7328
("inet: get rid of central tcp/dccp listener timer") Signed-off-by: Eric Dumazet <edumazet@google.com> Reported-by: Yuchung Cheng <ycheng@google.com> Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
parent
1d8dc3d3c8
commit
b357a364c5
@ -279,12 +279,6 @@ static inline void inet_csk_reqsk_queue_add(struct sock *sk,
|
||||
void inet_csk_reqsk_queue_hash_add(struct sock *sk, struct request_sock *req,
|
||||
unsigned long timeout);
|
||||
|
||||
static inline void inet_csk_reqsk_queue_removed(struct sock *sk,
|
||||
struct request_sock *req)
|
||||
{
|
||||
reqsk_queue_removed(&inet_csk(sk)->icsk_accept_queue, req);
|
||||
}
|
||||
|
||||
static inline void inet_csk_reqsk_queue_added(struct sock *sk,
|
||||
const unsigned long timeout)
|
||||
{
|
||||
@ -306,19 +300,7 @@ static inline int inet_csk_reqsk_queue_is_full(const struct sock *sk)
|
||||
return reqsk_queue_is_full(&inet_csk(sk)->icsk_accept_queue);
|
||||
}
|
||||
|
||||
static inline void inet_csk_reqsk_queue_unlink(struct sock *sk,
|
||||
struct request_sock *req)
|
||||
{
|
||||
reqsk_queue_unlink(&inet_csk(sk)->icsk_accept_queue, req);
|
||||
}
|
||||
|
||||
static inline void inet_csk_reqsk_queue_drop(struct sock *sk,
|
||||
struct request_sock *req)
|
||||
{
|
||||
inet_csk_reqsk_queue_unlink(sk, req);
|
||||
inet_csk_reqsk_queue_removed(sk, req);
|
||||
reqsk_put(req);
|
||||
}
|
||||
void inet_csk_reqsk_queue_drop(struct sock *sk, struct request_sock *req);
|
||||
|
||||
void inet_csk_destroy_sock(struct sock *sk);
|
||||
void inet_csk_prepare_forced_close(struct sock *sk);
|
||||
|
@ -212,24 +212,6 @@ static inline int reqsk_queue_empty(struct request_sock_queue *queue)
|
||||
return queue->rskq_accept_head == NULL;
|
||||
}
|
||||
|
||||
static inline void reqsk_queue_unlink(struct request_sock_queue *queue,
|
||||
struct request_sock *req)
|
||||
{
|
||||
struct listen_sock *lopt = queue->listen_opt;
|
||||
struct request_sock **prev;
|
||||
|
||||
spin_lock(&queue->syn_wait_lock);
|
||||
|
||||
prev = &lopt->syn_table[req->rsk_hash];
|
||||
while (*prev != req)
|
||||
prev = &(*prev)->dl_next;
|
||||
*prev = req->dl_next;
|
||||
|
||||
spin_unlock(&queue->syn_wait_lock);
|
||||
if (del_timer(&req->rsk_timer))
|
||||
reqsk_put(req);
|
||||
}
|
||||
|
||||
static inline void reqsk_queue_add(struct request_sock_queue *queue,
|
||||
struct request_sock *req,
|
||||
struct sock *parent,
|
||||
|
@ -453,7 +453,8 @@ static struct sock *dccp_v4_hnd_req(struct sock *sk, struct sk_buff *skb)
|
||||
iph->saddr, iph->daddr);
|
||||
if (req) {
|
||||
nsk = dccp_check_req(sk, skb, req);
|
||||
reqsk_put(req);
|
||||
if (!nsk)
|
||||
reqsk_put(req);
|
||||
return nsk;
|
||||
}
|
||||
nsk = inet_lookup_established(sock_net(sk), &dccp_hashinfo,
|
||||
|
@ -301,7 +301,8 @@ static struct sock *dccp_v6_hnd_req(struct sock *sk,struct sk_buff *skb)
|
||||
&iph->daddr, inet6_iif(skb));
|
||||
if (req) {
|
||||
nsk = dccp_check_req(sk, skb, req);
|
||||
reqsk_put(req);
|
||||
if (!nsk)
|
||||
reqsk_put(req);
|
||||
return nsk;
|
||||
}
|
||||
nsk = __inet6_lookup_established(sock_net(sk), &dccp_hashinfo,
|
||||
|
@ -186,8 +186,7 @@ struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb,
|
||||
if (child == NULL)
|
||||
goto listen_overflow;
|
||||
|
||||
inet_csk_reqsk_queue_unlink(sk, req);
|
||||
inet_csk_reqsk_queue_removed(sk, req);
|
||||
inet_csk_reqsk_queue_drop(sk, req);
|
||||
inet_csk_reqsk_queue_add(sk, req, child);
|
||||
out:
|
||||
return child;
|
||||
|
@ -564,6 +564,40 @@ int inet_rtx_syn_ack(struct sock *parent, struct request_sock *req)
|
||||
}
|
||||
EXPORT_SYMBOL(inet_rtx_syn_ack);
|
||||
|
||||
/* return true if req was found in the syn_table[] */
|
||||
static bool reqsk_queue_unlink(struct request_sock_queue *queue,
|
||||
struct request_sock *req)
|
||||
{
|
||||
struct listen_sock *lopt = queue->listen_opt;
|
||||
struct request_sock **prev;
|
||||
bool found = false;
|
||||
|
||||
spin_lock(&queue->syn_wait_lock);
|
||||
|
||||
for (prev = &lopt->syn_table[req->rsk_hash]; *prev != NULL;
|
||||
prev = &(*prev)->dl_next) {
|
||||
if (*prev == req) {
|
||||
*prev = req->dl_next;
|
||||
found = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
spin_unlock(&queue->syn_wait_lock);
|
||||
if (del_timer(&req->rsk_timer))
|
||||
reqsk_put(req);
|
||||
return found;
|
||||
}
|
||||
|
||||
void inet_csk_reqsk_queue_drop(struct sock *sk, struct request_sock *req)
|
||||
{
|
||||
if (reqsk_queue_unlink(&inet_csk(sk)->icsk_accept_queue, req)) {
|
||||
reqsk_queue_removed(&inet_csk(sk)->icsk_accept_queue, req);
|
||||
reqsk_put(req);
|
||||
}
|
||||
}
|
||||
EXPORT_SYMBOL(inet_csk_reqsk_queue_drop);
|
||||
|
||||
static void reqsk_timer_handler(unsigned long data)
|
||||
{
|
||||
struct request_sock *req = (struct request_sock *)data;
|
||||
|
@ -1348,7 +1348,8 @@ static struct sock *tcp_v4_hnd_req(struct sock *sk, struct sk_buff *skb)
|
||||
req = inet_csk_search_req(sk, th->source, iph->saddr, iph->daddr);
|
||||
if (req) {
|
||||
nsk = tcp_check_req(sk, skb, req, false);
|
||||
reqsk_put(req);
|
||||
if (!nsk)
|
||||
reqsk_put(req);
|
||||
return nsk;
|
||||
}
|
||||
|
||||
|
@ -755,10 +755,11 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
|
||||
if (!child)
|
||||
goto listen_overflow;
|
||||
|
||||
inet_csk_reqsk_queue_unlink(sk, req);
|
||||
inet_csk_reqsk_queue_removed(sk, req);
|
||||
|
||||
inet_csk_reqsk_queue_drop(sk, req);
|
||||
inet_csk_reqsk_queue_add(sk, req, child);
|
||||
/* Warning: caller must not call reqsk_put(req);
|
||||
* child stole last reference on it.
|
||||
*/
|
||||
return child;
|
||||
|
||||
listen_overflow:
|
||||
|
@ -946,7 +946,8 @@ static struct sock *tcp_v6_hnd_req(struct sock *sk, struct sk_buff *skb)
|
||||
&ipv6_hdr(skb)->daddr, tcp_v6_iif(skb));
|
||||
if (req) {
|
||||
nsk = tcp_check_req(sk, skb, req, false);
|
||||
reqsk_put(req);
|
||||
if (!nsk)
|
||||
reqsk_put(req);
|
||||
return nsk;
|
||||
}
|
||||
nsk = __inet6_lookup_established(sock_net(sk), &tcp_hashinfo,
|
||||
|
Loading…
Reference in New Issue
Block a user