| Message ID | 20240830080505.3545641-1-lihongbo22@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | [-next] scsi: qla2xxx: replace simple_strtoul to kstrtoul | expand |
On 2024/8/30 20:13, Bart Van Assche wrote: > On 8/30/24 4:05 AM, Hongbo Li wrote: >> - num_act_qp = simple_strtoul(buf, NULL, 0); >> + if (kstrtoul(buf, 0, &num_act_qp)) { >> + pr_err("host:%ld: fail to parse user buffer into number.", >> + vha->host_no); >> + rc = -EINVAL; >> + goto out_free; >> + } > > The message "fail to parse user buffer into number" is a bit long. > "failed to parse" is probably sufficient. > > Additionally, has it been considered to assign the kstrtoul() return > value to rc instead of discarding the returned value? kstrtoul can tell two error which are ERANGE and EINVAL. rc can keep return value from kstrtoul, and it won't affect the original code. Can I use rc to hold the return value from kstrtoul? Thanks, Hongbo > > Thanks, > > Bart.
diff --git a/drivers/scsi/qla2xxx/qla_dfs.c b/drivers/scsi/qla2xxx/qla_dfs.c index a1545dad0c0c..e92d4e43bdf5 100644 --- a/drivers/scsi/qla2xxx/qla_dfs.c +++ b/drivers/scsi/qla2xxx/qla_dfs.c @@ -598,7 +598,12 @@ qla_dfs_naqp_write(struct file *file, const char __user *buffer, return PTR_ERR(buf); } - num_act_qp = simple_strtoul(buf, NULL, 0); + if (kstrtoul(buf, 0, &num_act_qp)) { + pr_err("host:%ld: fail to parse user buffer into number.", + vha->host_no); + rc = -EINVAL; + goto out_free; + } if (num_act_qp >= vha->hw->max_qpairs) { pr_err("User set invalid number of qpairs %lu. Max = %d",
The function simple_strtoul performs no error checking in scenarios where the input value overflows the intended output variable. We can replace the use of the simple_strtoul with the safer alternatives kstrtoul. For fail case, we also print the extra message. Signed-off-by: Hongbo Li <lihongbo22@huawei.com> --- drivers/scsi/qla2xxx/qla_dfs.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)