Messages in this thread | | | Date | Mon, 29 Aug 2022 23:48:01 -0700 | Subject | Re: possible deadlock in rfcomm_sk_state_change | From | Desmond Cheong Zhi Xi <> |
| |
+cc Bluetooth and Networking maintainers
Hi Jiacheng,
On 28/8/22 04:03, Jiacheng Xu wrote: > Hi, > > I believe the deadlock is more than possible but actually real. > I got a poc that could stably trigger the deadlock. > > poc: https://drive.google.com/file/d/1PjqvMtHsrrGM1MIRGKl_zJGR-teAMMQy/view?usp=sharing > > Description/Root cause: > In rfcomm_sock_shutdown(), lock_sock() is called when releasing and > shutting down socket. > However, lock_sock() has to be called once more when the sk_state is > changed because the > lock is not always held when rfcomm_sk_state_change() is called. One > such call stack is: > > rfcomm_sock_shutdown(): > lock_sock(); > __rfcomm_sock_close(): > rfcomm_dlc_close(): > __rfcomm_dlc_close(): > rfcomm_dlc_lock(); > rfcomm_sk_state_change(): > lock_sock(); > > Besides the recursive deadlock, there is also an > issue of a lock hierarchy inversion between rfcomm_dlc_lock() and > lock_sock() if the socket is locked in rfcomm_sk_state_change().
Thanks for the poc and for following the trail all the way to the root cause - this was a known issue and I didn't realize the patch wasn't applied.
> > Reference: https://lore.kernel.org/all/20211004180734.434511-1-desmondcheongzx@gmail.com/ >
Fwiw, I tested the patch again with syzbot. It still applies cleanly to the head of bluetooth-next and seems to address the root cause.
Any thoughts from the maintainers on this issue and the proposed fix?
Best, Desmond
| |