Now that scatter_elem_sz is validated and rounded up to a power-of-two multiple of PAGE_SIZE by the .set callback at write time (and the init-time value SG_SCATTER_SZ is always >= PAGE_SIZE), the runtime fixup logic in sg_build_indirect() is redundant: - The entry fixup (syncing scatter_elem_sz_prev to scatter_elem_sz) was fragile -- it updated scatter_elem_sz_prev but not the local variable "num", which is the exact source of the UBSAN warning fixed by the previous patch. - The in-loop fixup (updating scatter_elem_sz to ret_sz when ret_sz > scatter_elem_sz_prev) is no longer needed because scatter_elem_sz is always a power-of-two multiple of PAGE_SIZE, making it always equal to ret_sz. - The init_sg boot-time clamp is also removed as it can no longer trigger (SG_SCATTER_SZ >= PAGE_SIZE, and the .set callback rejects sub-PAGE_SIZE values). Remove the scatter_elem_sz_prev variable, the entry/in-loop fixup, and the boot-time clamp. Snapshot scatter_elem_sz into a local elem_sz for get_order(), and switch the debug printk to "%s"/__func__ since the "num" argument no longer exists. While at it, replace S_IRUGO | S_IWUSR with 0644 for the allow_dio and def_reserved_size module parameters, as checkpatch warns about symbolic permissions on touched module_param lines. Signed-off-by: Yang Erkun --- drivers/scsi/sg.c | 40 +++++++--------------------------------- 1 file changed, 7 insertions(+), 33 deletions(-) diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index c56f8460c03f..377c3e1dee7f 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -92,7 +92,6 @@ static int def_reserved_size = SG_DEF_RESERVED_SIZE; static int sg_allow_dio = SG_ALLOW_DIO_DEF; static int scatter_elem_sz = SG_SCATTER_SZ; -static int scatter_elem_sz_prev = SG_SCATTER_SZ; #define SG_SECTOR_SZ 512 @@ -1649,7 +1648,7 @@ static int scatter_elem_sz_set(const char *val, const struct kernel_param *kp) module_param_call(scatter_elem_sz, scatter_elem_sz_set, param_get_int, &scatter_elem_sz, 0644); -module_param_named(allow_dio, sg_allow_dio, int, S_IRUGO | S_IWUSR); +module_param_named(allow_dio, sg_allow_dio, int, 0644); static int def_reserved_size_set(const char *val, const struct kernel_param *kp) { @@ -1675,8 +1674,7 @@ static const struct kernel_param_ops def_reserved_size_ops = { .get = param_get_int, }; -module_param_cb(def_reserved_size, &def_reserved_size_ops, &def_reserved_size, - S_IRUGO | S_IWUSR); +module_param_cb(def_reserved_size, &def_reserved_size_ops, &def_reserved_size, 0644); MODULE_AUTHOR("Douglas Gilbert"); MODULE_DESCRIPTION("SCSI generic (sg) driver"); @@ -1694,11 +1692,6 @@ init_sg(void) { int rc; - if (scatter_elem_sz < PAGE_SIZE) { - scatter_elem_sz = PAGE_SIZE; - scatter_elem_sz_prev = scatter_elem_sz; - } - rc = register_chrdev_region(MKDEV(SCSI_GENERIC_MAJOR, 0), SG_MAX_DEVS, "sg"); if (rc) @@ -1880,9 +1873,10 @@ sg_build_sgat(Sg_scatter_hold * schp, const Sg_fd * sfp, int tablesize) static int sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp, int buff_size) { - int ret_sz = 0, i, k, rem_sz, num, mx_sc_elems; + int ret_sz = 0, i, k, rem_sz, mx_sc_elems; int sg_tablesize = sfp->parentdp->sg_tablesize; int blk_size = buff_size, order; + int elem_sz = scatter_elem_sz; gfp_t gfp_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN | __GFP_ZERO; if (blk_size < 0) @@ -1900,39 +1894,19 @@ sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp, int buff_size) if (mx_sc_elems < 0) return mx_sc_elems; /* most likely -ENOMEM */ - num = scatter_elem_sz; - if (unlikely(num != scatter_elem_sz_prev)) { - if (num < PAGE_SIZE) { - scatter_elem_sz = PAGE_SIZE; - scatter_elem_sz_prev = PAGE_SIZE; - } else - scatter_elem_sz_prev = num; - } - - order = get_order(num); + order = get_order(elem_sz); retry: ret_sz = 1 << (PAGE_SHIFT + order); for (k = 0, rem_sz = blk_size; rem_sz > 0 && k < mx_sc_elems; k++, rem_sz -= ret_sz) { - - num = (rem_sz > scatter_elem_sz_prev) ? - scatter_elem_sz_prev : rem_sz; - schp->pages[k] = alloc_pages(gfp_mask, order); if (!schp->pages[k]) goto out; - if (num == scatter_elem_sz_prev) { - if (unlikely(ret_sz > scatter_elem_sz_prev)) { - scatter_elem_sz = ret_sz; - scatter_elem_sz_prev = ret_sz; - } - } - SCSI_LOG_TIMEOUT(5, sg_printk(KERN_INFO, sfp->parentdp, - "sg_build_indirect: k=%d, num=%d, ret_sz=%d\n", - k, num, ret_sz)); + "%s: k=%d, ret_sz=%d\n", + __func__, k, ret_sz)); } /* end of for loop */ schp->page_order = order; -- 2.52.0