Writing an invalid value such as 0 or a negative number to
/sys/module/sg/parameters/scatter_elem_sz causes a shift-out-of-bounds bug
during subsequent device operations. 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 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:
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
Fix this by casting PAGE_SIZE to int so that the comparison is 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 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=e273ef4c-bec0-4d7e-af59-32e327619af1
To: "James E.J. Bottomley"
To: "Doug Gilbert"
To:
To: "Martin K. Petersen"
To: "FUJITA Tomonori"
Cc:
---
v2:
- Drop module_param_cb() setter callback and range validation at sysfs boundary.
- Cast PAGE_SIZE to int in sg_build_indirect() to prevent negative values from bypassing the check.
- Update local snapshot variable num to PAGE_SIZE when num is less than PAGE_SIZE.
- Update commit description to explain the unsigned comparison and unupdated local variable, and remove claims regarding parameter data races.
v1:
https://lore.kernel.org/all/30c58052-61b7-4425-9fd7-c188a0bef4ea@mail.kernel.org/T/
---
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 5408f002e..39c8e52fa 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -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
--
This is an AI-generated patch subject to moderation.
Reply with '#syz upstream' to Sign-off the patch as a human author
and send it to the upstream kernel mailing lists.
Reply with '#syz reject' to reject it ('#syz unreject' to undo).
See https://goo.gle/syzbot-ai-patches for information about AI-generated patches.
You can comment on the patch as usual, syzbot will try to address
the comments and send a new version of the patch if necessary.
syzbot engineers can be reached at syzkaller@googlegroups.com.