scsi: sg: fix slab-use-after-free Read in sg_release

Commit Message

Suraj Sonawane Nov. 20, 2024, 12:59 p.m. UTC

Fix a use-after-free bug in `sg_release`,
detected by syzbot with KASAN:

BUG: KASAN: slab-use-after-free in lock_release+0x151/0xa30
kernel/locking/lockdep.c:5838
__mutex_unlock_slowpath+0xe2/0x750 kernel/locking/mutex.c:912
sg_release+0x1f4/0x2e0 drivers/scsi/sg.c:407

Root Cause:
In `sg_release`, the function `kref_put(&sfp->f_ref, sg_remove_sfp)`
is called before releasing the `open_rel_lock` mutex. The `kref_put`
call may decrement the reference count of `sfp` to zero, triggering
its cleanup through `sg_remove_sfp`. This cleanup includes scheduling
deferred work via `sg_remove_sfp_usercontext`, which ultimately frees
`sfp`.

After `kref_put`, `sg_release` continues to unlock `open_rel_lock` and
may reference `sfp` or `sdp`. If `sfp` has already been freed, this
results in a slab-use-after-free error.

Fix:
The `kref_put(&sfp->f_ref, sg_remove_sfp)` call is moved after unlocking
the `open_rel_lock` mutex. This ensures:
- No references to `sfp` or `sdp` occur after the reference count is  
  decremented.  
- Cleanup functions such as sg_remove_sfp and sg_remove_sfp_usercontext  
  can safely execute without impacting the mutex handling in `sg_release`. 

The fix has been tested and validated by syzbot. This patch closes the bug
reported at the following syzkaller link and ensures proper sequencing of
resource cleanup and mutex operations, eliminating the risk of
use-after-free errors in `sg_release`.

Reported-by: [e-mail] 
Closes: https://syzkaller.appspot.com/bug?extid=7efb5850a17ba6ce098b 
Tested-by: [e-mail] 
Fixes: cc833acbee9d ("sg: O_EXCL and other lock handling ")
Signed-off-by: Suraj Sonawane <[e-mail]>
---
 drivers/scsi/sg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index f86be197f..457d54171 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -393,7 +393,6 @@  sg_release(struct inode *inode, struct file *filp)
 
 	mutex_lock(&sdp->open_rel_lock);
 	scsi_autopm_put_device(sdp->device);
-	kref_put(&sfp->f_ref, sg_remove_sfp);
 	sdp->open_cnt--;
 
 	/* possibly many open()s waiting on exlude clearing, start many;
@@ -405,6 +404,7 @@  sg_release(struct inode *inode, struct file *filp)
 		wake_up_interruptible(&sdp->open_wait);
 	}
 	mutex_unlock(&sdp->open_rel_lock);
+	kref_put(&sfp->f_ref, sg_remove_sfp);
 	return 0;
 }