Skip to contentSkip to conversation

mvsas: don't allow negative timeouts

PATCH Applied Dan Carpenter 3 participants 3 messages Nov 13, 2015

Subsystems (2)
SCSI Kernel janitors
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 mvsas: don't allow negative timeouts

There is a static checker warning here because "val" is controlled by the user and we have a upper bound on it but allow negative numbers. "val" appears to be a timeout in usec so this bug probably means we have a longer timeout than we should. Let's fix this by changing "val" to unsigned.

Signed-off-by Dan Carpenter <[email protected]>

---
Checkpatch has several complaints about this code but I left it as-is.

drivers/scsi/mvsas/mv_init.c +2 -2
index 90fdf0e..675e7fa 100644
@@ -758,7 +758,7 @@ mvs_store_interrupt_coalescing(struct device *cdev,
 			struct device_attribute *attr,
 			const char *buffer, size_t size)
 {
-	int val = 0;
+	unsigned int val = 0;
 	struct mvs_info *mvi = NULL;
 	struct Scsi_Host *shost = class_to_shost(cdev);
 	struct sas_ha_struct *sha = SHOST_TO_SAS_HA(shost);

@@ -766,7 +766,7 @@ mvs_store_interrupt_coalescing(struct device *cdev,
 	if (buffer = NULL)
 		return size;
 
-	if (sscanf(buffer, "%d", &val) != 1)
+	if (sscanf(buffer, "%u", &val) != 1)
 		return -EINVAL;
 
 	if (val >= 0x10000) {
Reviewed-by on v1 1/1
#1 lore raw
Quoted 24 lines · Dan Carpenter

There is a static checker warning here because "val" is controlled by the user and we have a upper bound on it but allow negative numbers. "val" appears to be a timeout in usec so this bug probably means we have a longer timeout than we should. Let's fix this by changing "val" to unsigned.

Signed-off-by Dan Carpenter <[email protected]>

---
Checkpatch has several complaints about this code but I left it as-is.

drivers/scsi/mvsas/mv_init.c +1 -1
index 90fdf0e..675e7fa 100644
@@ -758,7 +758,7 @@ mvs_store_interrupt_coalescing(struct device *cdev,
                        struct device_attribute *attr,
                        const char *buffer, size_t size)
 {
-       int val = 0;
+       unsigned int val = 0;
        struct mvs_info *mvi = NULL;
        struct Scsi_Host *shost = class_to_shost(cdev);
        struct sas_ha_struct *sha = SHOST_TO_SAS_HA(shost);

drivers/scsi/mvsas/mv_init.c mvs_store_interrupt_coalescing(struct device *cdev, · line 772

Quoted 8 lines
drivers/scsi/mvsas/mv_init.c +1 -1
@@ -766,7 +766,7 @@ mvs_store_interrupt_coalescing(struct device *cdev,
        if (buffer = NULL)
                return size;
 
-       if (sscanf(buffer, "%d", &val) != 1)
+       if (sscanf(buffer, "%u", &val) != 1)
                return -EINVAL;
 
        if (val >= 0x10000) {

Looks good to me.

Reviewed-by Jack Wang <[email protected]>
Signature · 7 lines
Mit freundlichen Grüßen,
Best Regards,

Jack Wang

Linux Kernel Developer Storage
ProfitBricks GmbH  The IaaS-Company.
Applied
#2 lore raw
In reply to Dan Carpenter
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Dan" = Dan Carpenter <[email protected]> writes:

Dan> There is a static checker warning here because "val" is controlled Dan> by the user and we have a upper bound on it but allow negative Dan> numbers. "val" appears to be a timeout in usec so this bug Dan> probably means we have a longer timeout than we should. Let's fix Dan> this by changing "val" to unsigned.

Applied.

Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering