On 11/20/24 18:29, Suraj Sonawane wrote:
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: syzbot+7efb5850a17ba6ce098b@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=7efb5850a17ba6ce098b
Tested-by: syzbot+7efb5850a17ba6ce098b@xxxxxxxxxxxxxxxxxxxxxxxxx
Fixes: cc833acbee9d ("sg: O_EXCL and other lock handling ")
Signed-off-by: Suraj Sonawane <surajsonawane0215@xxxxxxxxx>
---
drivers/scsi/sg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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;
}
Hello!
I wanted to follow up on the patch I submitted. I was wondering if you
had a chance to review it and if there are any comments or feedback.
Thank you for your time and consideration. I look forward to your response.
Best regards,
Suraj Sonawane
