[PATCH] scsi: sg: Fix shift-out-of-bounds in sg_build_indirect()

[PATCH] scsi: sg: Fix shift-out-of-bounds in sg_build_indirect()

[syzbot]

From: Krystian Kaniewski <krystianmkaniewski@gmail.com>

Writing an invalid value such as 0 or a negative number to
/sys/module/sg/parameters/scatter_elem_sz, or setting it at module load
time, causes a shift-out-of-bounds bug during subsequent device operations.
In init_sg(), scatter_elem_sz is checked against PAGE_SIZE, but because
PAGE_SIZE is unsigned, the comparison is unsigned and bypasses negative
values. When opening a device via sg_open(), the driver calls
sg_build_indirect() to allocate scatter-gather buffers. In
sg_build_indirect(), scatter_elem_sz is read into the local snapshot
variable num. Because PAGE_SIZE is unsigned, comparing num against
PAGE_SIZE also results in an unsigned comparison that bypasses negative
values. Furthermore, even if the condition is met, the code updates the
global scatter_elem_sz and scatter_elem_sz_prev variables but leaves the
local snapshot variable num unupdated before get_order(). As a result, an
invalid value such as 0 remains in num and is passed to order =
get_order(num). Calling get_order(0) is undefined and underflows to 52 on
64-bit systems, causing ret_sz = 1 << (PAGE_SHIFT + order) to compute 1 <<
64. Shifting a 32-bit int by 64 bits triggers a UBSAN shift-out-of-bounds
warning:

UBSAN: shift-out-of-bounds in drivers/scsi/sg.c:1887:13
shift exponent 64 is too large for 32-bit type 'int'
Call Trace:
 <TASK>
 dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
 ubsan_epilogue+0xa/0x30 lib/ubsan.c:233
 __ubsan_handle_shift_out_of_bounds+0x36d/0x400 lib/ubsan.c:494
 sg_build_indirect+0x559/0x870 drivers/scsi/sg.c:1887
 sg_build_reserve drivers/scsi/sg.c:1997 [inline]
 sg_add_sfp drivers/scsi/sg.c:2177 [inline]
 sg_open+0x1128/0x18a0 drivers/scsi/sg.c:349
 chrdev_open+0x4d9/0x600 fs/char_dev.c:411
 do_dentry_open+0x816/0x1380 fs/open.c:996
 vfs_open+0x3b/0x340 fs/open.c:1101
 </TASK>

Fix this by casting PAGE_SIZE to int in both init_sg() and
sg_build_indirect() so that the comparisons are signed and negative values
do not bypass the check, and by updating the local snapshot variable num to
PAGE_SIZE when num < (int)PAGE_SIZE in sg_build_indirect() before
get_order() is called.

Fixes: 10db10d144c0 ("sg: convert the indirect IO path to use the block layer")
Assisted-by: Gemini:gemini-3.8-flash syzbot
Reported-by: syzbot+270f1c719ee7baab9941@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=270f1c719ee7baab9941
Link: https://syzkaller.appspot.com/ai_job?id=1d412b83-5bbc-4d73-b690-4c798d45cee4
Signed-off-by: Krystian Kaniewski <krystianmkaniewski@gmail.com>

---
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 5408f002e..46117d1a1 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1667,7 +1667,7 @@ init_sg(void)
 {
 	int rc;
 
-	if (scatter_elem_sz < PAGE_SIZE) {
+	if (scatter_elem_sz < (int)PAGE_SIZE) {
 		scatter_elem_sz = PAGE_SIZE;
 		scatter_elem_sz_prev = scatter_elem_sz;
 	}
@@ -1875,9 +1875,10 @@ sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp, int buff_size)
 
 	num = scatter_elem_sz;
 	if (unlikely(num != scatter_elem_sz_prev)) {
-		if (num < PAGE_SIZE) {
+		if (num < (int)PAGE_SIZE) {
 			scatter_elem_sz = PAGE_SIZE;
 			scatter_elem_sz_prev = PAGE_SIZE;
+			num = PAGE_SIZE;
 		} else
 			scatter_elem_sz_prev = num;
 	}


base-commit: df2908090cda368b01ff43709f51890076c56157
-- 
See https://goo.gle/syzbot-ai-patches for information about AI-generated patches.
The person who has signed off on the patch is responsible for
addressing comments.
syzbot engineers can be reached at syzkaller@googlegroups.com.