Message ID | CAH2r5mtJuttkzHDQB=-U3o=bBnv5U9r2w+JG-mXg1TPBT1wFog@mail.gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [SMB3,client] fix potential deadlock in cifs_sync_mid_result | expand |
On Wed, Apr 24, 2024 at 9:16 AM Steve French <smfrench@gmail.com> wrote: > > Coverity spotted that the cifs_sync_mid_result function could deadlock > since cifs_server_dbg graps the srv_lock while we are still holding > the mid_lock > > "Thread deadlock (ORDER_REVERSAL) lock_order: Calling spin_lock acquires > lock TCP_Server_Info.srv_lock while holding lock TCP_Server_Info.mid_lock" > > Addresses-Coverity: 1590401 ("Thread deadlock (ORDER_REVERSAL)") > > See attached patch > > > -- > Thanks, > > Steve Looks good to me.
Minor update to patch (shrink slightly by using a goto) On Thu, Apr 25, 2024 at 12:44 PM Shyam Prasad N <nspmangalore@gmail.com> wrote: > > On Wed, Apr 24, 2024 at 9:16 AM Steve French <smfrench@gmail.com> wrote: > > > > Coverity spotted that the cifs_sync_mid_result function could deadlock > > since cifs_server_dbg graps the srv_lock while we are still holding > > the mid_lock > > > > "Thread deadlock (ORDER_REVERSAL) lock_order: Calling spin_lock acquires > > lock TCP_Server_Info.srv_lock while holding lock TCP_Server_Info.mid_lock" > > > > Addresses-Coverity: 1590401 ("Thread deadlock (ORDER_REVERSAL)") > > > > See attached patch > > > > > > -- > > Thanks, > > > > Steve > > Looks good to me. > > -- > Regards, > Shyam
On Thu, Apr 25, 2024 at 11:23 PM Steve French <smfrench@gmail.com> wrote: > > Minor update to patch (shrink slightly by using a goto) > > > On Thu, Apr 25, 2024 at 12:44 PM Shyam Prasad N <nspmangalore@gmail.com> wrote: > > > > On Wed, Apr 24, 2024 at 9:16 AM Steve French <smfrench@gmail.com> wrote: > > > > > > Coverity spotted that the cifs_sync_mid_result function could deadlock > > > since cifs_server_dbg graps the srv_lock while we are still holding > > > the mid_lock > > > > > > "Thread deadlock (ORDER_REVERSAL) lock_order: Calling spin_lock acquires > > > lock TCP_Server_Info.srv_lock while holding lock TCP_Server_Info.mid_lock" > > > > > > Addresses-Coverity: 1590401 ("Thread deadlock (ORDER_REVERSAL)") > > > > > > See attached patch > > > > > > > > > -- > > > Thanks, > > > > > > Steve > > > > Looks good to me. > > > > -- > > Regards, > > Shyam > > > > -- > Thanks, > > Steve This one looks even better. Thanks.
From 9b42329261067a500f2452f131c88c8cb0b62aa5 Mon Sep 17 00:00:00 2001 From: Steve French <stfrench@microsoft.com> Date: Tue, 23 Apr 2024 22:35:28 -0500 Subject: [PATCH] smb3: fix lock ordering potential deadlock in cifs_sync_mid_result Coverity spotted that the cifs_sync_mid_result function could deadlock "Thread deadlock (ORDER_REVERSAL) lock_order: Calling spin_lock acquires lock TCP_Server_Info.srv_lock while holding lock TCP_Server_Info.mid_lock" Addresses-Coverity: 1590401 ("Thread deadlock (ORDER_REVERSAL)") Cc: stable@vger.kernel.org Signed-off-by: Steve French <stfrench@microsoft.com> --- fs/smb/client/transport.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c index 994d70193432..443b4b89431d 100644 --- a/fs/smb/client/transport.c +++ b/fs/smb/client/transport.c @@ -909,9 +909,11 @@ cifs_sync_mid_result(struct mid_q_entry *mid, struct TCP_Server_Info *server) list_del_init(&mid->qhead); mid->mid_flags |= MID_DELETED; } + spin_unlock(&server->mid_lock); cifs_server_dbg(VFS, "%s: invalid mid state mid=%llu state=%d\n", __func__, mid->mid, mid->mid_state); - rc = -EIO; + release_mid(mid); + return -EIO; } spin_unlock(&server->mid_lock); -- 2.40.1