On Fri, Mar 3, 2017 at 5:00 PM, Eric Dumazet <eric.dumazet@xxxxxxxxx> wrote:
> On Fri, 2017-03-03 at 07:22 -0800, Eric Dumazet wrote:
>> On Fri, Mar 3, 2017 at 7:12 AM, Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote:
>> > The first bot that picked this up started spewing:
>> >
>> > BUG: spinlock recursion on CPU#1, syz-executor2/9452
>>
>> Yes. The bug is not about locking the listener, but protecting fields
>> of struct dccp_request_sock
>>
>> I will provide a patch, once I reach the office and after the breakfast ;)
>
> OK here is what I suggest to fix the races.
I've applied the patch 2 days ago and it stopped happening since then.
Please mail an official patch.
Tested-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
> diff --git a/include/linux/dccp.h b/include/linux/dccp.h
> index 61d042bbbf607253033d9948b291cab2322814ba..68449293c4b6233c1a1d4133b1819376a9310225 100644
> --- a/include/linux/dccp.h
> +++ b/include/linux/dccp.h
> @@ -163,6 +163,7 @@ struct dccp_request_sock {
> __u64 dreq_isr;
> __u64 dreq_gsr;
> __be32 dreq_service;
> + spinlock_t dreq_lock;
> struct list_head dreq_featneg;
> __u32 dreq_timestamp_echo;
> __u32 dreq_timestamp_time;
> diff --git a/net/dccp/minisocks.c b/net/dccp/minisocks.c
> index e267e6f4c9a5566b369a03a600a408e5bd41cbad..abd07a443219853b022bef41cb072e90ff8f07f0 100644
> --- a/net/dccp/minisocks.c
> +++ b/net/dccp/minisocks.c
> @@ -142,6 +142,13 @@ struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb,
> struct dccp_request_sock *dreq = dccp_rsk(req);
> bool own_req;
>
> + /* TCP/DCCP listeners became lockless.
> + * DCCP stores complex state in its request_sock, so we need
> + * a protection for them, now this code runs without being protected
> + * by the parent (listener) lock.
> + */
> + spin_lock_bh(&dreq->dreq_lock);
> +
> /* Check for retransmitted REQUEST */
> if (dccp_hdr(skb)->dccph_type == DCCP_PKT_REQUEST) {
>
> @@ -156,7 +163,7 @@ struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb,
> inet_rtx_syn_ack(sk, req);
> }
> /* Network Duplicate, discard packet */
> - return NULL;
> + goto out;
> }
>
> DCCP_SKB_CB(skb)->dccpd_reset_code = DCCP_RESET_CODE_PACKET_ERROR;
> @@ -182,20 +189,20 @@ struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb,
>
> child = inet_csk(sk)->icsk_af_ops->syn_recv_sock(sk, skb, req, NULL,
> req, &own_req);
> - if (!child)
> - goto listen_overflow;
> -
> - return inet_csk_complete_hashdance(sk, child, req, own_req);
> + if (child) {
> + child = inet_csk_complete_hashdance(sk, child, req, own_req);
> + goto out;
> + }
>
> -listen_overflow:
> - dccp_pr_debug("listen_overflow!\n");
> DCCP_SKB_CB(skb)->dccpd_reset_code = DCCP_RESET_CODE_TOO_BUSY;
> drop:
> if (dccp_hdr(skb)->dccph_type != DCCP_PKT_RESET)
> req->rsk_ops->send_reset(sk, skb);
>
> inet_csk_reqsk_queue_drop(sk, req);
> - return NULL;
> +out:
> + spin_unlock_bh(&dreq->dreq_lock);
> + return child;
> }
>
> EXPORT_SYMBOL_GPL(dccp_check_req);
> @@ -246,6 +253,7 @@ int dccp_reqsk_init(struct request_sock *req,
> {
> struct dccp_request_sock *dreq = dccp_rsk(req);
>
> + spin_lock_init(&dreq->dreq_lock);
> inet_rsk(req)->ir_rmt_port = dccp_hdr(skb)->dccph_sport;
> inet_rsk(req)->ir_num = ntohs(dccp_hdr(skb)->dccph_dport);
> inet_rsk(req)->acked = 0;
>
>
--
To unsubscribe from this list: send the line "unsubscribe dccp" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
