[PATCH v2 2/2] scsi: sg: clean up scatter_elem_sz handling and module param permissions

[Yang Erkun <yangerkun@huawei.com>]

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 <yangerkun@huawei.com>
---
 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