[PATCH] scsi: csiostor: fix inverted sscanf() check in csio_store_dbg_level

[Rodrigo Gobbi]

csio_store_dbg_level() has an inverted return check on sscanf():
the existing check treats a truthy return as an error, so every
valid write to the "dbg_level" sysfs attribute was rejected with
-EINVAL. This has been broken since the driver was first added.

checkpatch also flags this same line for a related issue:

WARNING: unchecked sscanf return value
WARNING: Prefer kstrto<type> to single variable sscanf

Replace the sscanf()/isdigit() combo with kstrtou32(buf, 16, ...),
so this class of inversion bug can't happen again. It also validates
the whole string instead of stopping at the first non-digit. This makes
the isdigit() pre-check redundant, so it's removed along with the now
unused <linux/ctype.h> include.

Base 16 is used instead of base 0 (which would only recognize hex
with an explicit "0x" prefix) to match csio_show_dbg_level(),
which always prints the value as bare hex via "%x". This keeps
read and write symmetric: writing back what was just read always
restores the same value.

Fixes: a3667aaed569 ("[SCSI] csiostor: Chelsio FCoE offload driver")
Signed-off-by: Rodrigo Gobbi <rodrigo.gobbi.7@xxxxxxxxx>
---
Hi all,

While looking at this file I noticed checkpatch flags
csio_store_dbg_level() with:

WARNING: unchecked sscanf return value
WARNING: Prefer kstrto<type> to single variable sscanf

Digging into it, the sscanf() check is actually inverted, so
csio_store_dbg_level() has been rejecting every valid write since the
driver was added. This patch fixes that with kstrtou32(), using base
16 (instead of 0) so that copy-pasting the value printed by
csio_show_dbg_level() (which is always printed as bare hex) back into
the store function round-trips correctly.

While tracing this I also noticed that log_level doesn't seem to be
read anywhere else in the driver to gate any behaviour -- I couldn't
find a consumer for it. I'm not proposing to remove the dbg_level
sysfs attribute as part of this fix, just flagging it in case it's
dead code worth a separate cleanup, if the maintainers agree.

I wasn't able to test this patch, only compile it.
Thanks and regards.
---
 drivers/scsi/csiostor/csio_scsi.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/csiostor/csio_scsi.c b/drivers/scsi/csiostor/csio_scsi.c
index b1de615cf316..d64b47ee1bbe 100644
--- a/drivers/scsi/csiostor/csio_scsi.c
+++ b/drivers/scsi/csiostor/csio_scsi.c
@@ -34,7 +34,6 @@
 
 #include <linux/device.h>
 #include <linux/delay.h>
-#include <linux/ctype.h>
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/string.h>
@@ -1441,12 +1440,11 @@ csio_store_dbg_level(struct device *dev,
 	struct csio_lnode *ln = shost_priv(class_to_shost(dev));
 	struct csio_hw *hw = csio_lnode_to_hw(ln);
 	uint32_t dbg_level = 0;
+	int ret;
 
-	if (!isdigit(buf[0]))
-		return -EINVAL;
-
-	if (sscanf(buf, "%i", &dbg_level))
-		return -EINVAL;
+	ret = kstrtou32(buf, 16, &dbg_level);
+	if (ret)
+		return ret;
 
 	ln->params.log_level = dbg_level;
 	hw->params.log_level = dbg_level;
-- 
2.48.1