netdev - Re: [Patch bpf v2] sock_map: move cancel_work

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87eduk43i5.fsf@cloudflare.com>
Date:   Thu, 03 Nov 2022 13:37:36 +0100
From:   Jakub Sitnicki <jakub@...udflare.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     netdev@...r.kernel.org, bpf@...r.kernel.org,
        Cong Wang <cong.wang@...edance.com>,
        Stanislav Fomichev <sdf@...gle.com>,
        John Fastabend <john.fastabend@...il.com>
Subject: Re: [Patch bpf v2] sock_map: move cancel_work_sync() out of sock lock

On Tue, Nov 01, 2022 at 09:34 PM -07, Cong Wang wrote:
> From: Cong Wang <cong.wang@...edance.com>
>
> Stanislav reported a lockdep warning, which is caused by the
> cancel_work_sync() called inside sock_map_close(), as analyzed
> below by Jakub:
>
> psock->work.func = sk_psock_backlog()
>   ACQUIRE psock->work_mutex
>     sk_psock_handle_skb()
>       skb_send_sock()
>         __skb_send_sock()
>           sendpage_unlocked()
>             kernel_sendpage()
>               sock->ops->sendpage = inet_sendpage()
>                 sk->sk_prot->sendpage = tcp_sendpage()
>                   ACQUIRE sk->sk_lock
>                     tcp_sendpage_locked()
>                   RELEASE sk->sk_lock
>   RELEASE psock->work_mutex
>
> sock_map_close()
>   ACQUIRE sk->sk_lock
>   sk_psock_stop()
>     sk_psock_clear_state(psock, SK_PSOCK_TX_ENABLED)
>     cancel_work_sync()
>       __cancel_work_timer()
>         __flush_work()
>           // wait for psock->work to finish
>   RELEASE sk->sk_lock
>
> We can move the cancel_work_sync() out of the sock lock protection,
> but still before saved_close() was called.
>
> Fixes: 799aa7f98d53 ("skmsg: Avoid lock_sock() in sk_psock_backlog()")
> Reported-by: Stanislav Fomichev <sdf@...gle.com>
> Cc: John Fastabend <john.fastabend@...il.com>
> Cc: Jakub Sitnicki <jakub@...udflare.com>
> Signed-off-by: Cong Wang <cong.wang@...edance.com>
> ---

[...]

Thanks!

Acked-by: Jakub Sitnicki <jakub@...udflare.com>
Tested-by: Jakub Sitnicki <jakub@...udflare.com>