scsi: fcoe: Fix unsigned comparison with zero in store_ctlr_mode()

Message ID 20240102085245.600570-1-harshit.m.mogalapalli@oracle.com
State New
Headers
Series scsi: fcoe: Fix unsigned comparison with zero in store_ctlr_mode() |

Commit Message

Harshit Mogalapalli Jan. 2, 2024, 8:52 a.m. UTC
  ctlr->mode is of unsigned type, it is never less than zero.

Fix this by using an extra varibale called 'res', to store return value
from sysfs_match_string() and assign that to ctlr->mode on the success
path.

Fixes: edc22a7c8688 ("scsi: fcoe: Use sysfs_match_string() over fcoe_parse_mode()")
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
This is based on static analysis with smatch and only compile tested.
---
 drivers/scsi/fcoe/fcoe_sysfs.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
  

Comments

Hannes Reinecke Jan. 8, 2024, 12:31 p.m. UTC | #1
On 1/2/24 09:52, Harshit Mogalapalli wrote:
> ctlr->mode is of unsigned type, it is never less than zero.
> 
> Fix this by using an extra varibale called 'res', to store return value
> from sysfs_match_string() and assign that to ctlr->mode on the success
> path.
> 
> Fixes: edc22a7c8688 ("scsi: fcoe: Use sysfs_match_string() over fcoe_parse_mode()")
> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> ---
> This is based on static analysis with smatch and only compile tested.
> ---
>   drivers/scsi/fcoe/fcoe_sysfs.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/fcoe/fcoe_sysfs.c b/drivers/scsi/fcoe/fcoe_sysfs.c
> index 408a806bf4c2..c64a085a7ee2 100644
> --- a/drivers/scsi/fcoe/fcoe_sysfs.c
> +++ b/drivers/scsi/fcoe/fcoe_sysfs.c
> @@ -263,6 +263,7 @@ static ssize_t store_ctlr_mode(struct device *dev,
>   			       const char *buf, size_t count)
>   {
>   	struct fcoe_ctlr_device *ctlr = dev_to_ctlr(dev);
> +	int res;
>   
>   	if (count > FCOE_MAX_MODENAME_LEN)
>   		return -EINVAL;
> @@ -279,12 +280,13 @@ static ssize_t store_ctlr_mode(struct device *dev,
>   			return -ENOTSUPP;
>   		}
>   
> -		ctlr->mode = sysfs_match_string(fip_conn_type_names, buf);
> -		if (ctlr->mode < 0 || ctlr->mode == FIP_CONN_TYPE_UNKNOWN) {
> +		res = sysfs_match_string(fip_conn_type_names, buf);
> +		if (res < 0 || res == FIP_CONN_TYPE_UNKNOWN) {
>   			LIBFCOE_SYSFS_DBG(ctlr, "Unknown mode %s provided.\n",
>   					  buf);
>   			return -EINVAL;
>   		}
> +		ctlr->mode = res;
>   
>   		ctlr->f->set_fcoe_ctlr_mode(ctlr);
>   		LIBFCOE_SYSFS_DBG(ctlr, "Mode changed to %s.\n", buf);
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
  
Martin K. Petersen Jan. 12, 2024, 2:27 a.m. UTC | #2
Harshit,

> ctlr->mode is of unsigned type, it is never less than zero.

Applied to 6.8/scsi-staging, thanks!
  
Martin K. Petersen Jan. 17, 2024, 8:10 p.m. UTC | #3
On Tue, 02 Jan 2024 00:52:45 -0800, Harshit Mogalapalli wrote:

> ctlr->mode is of unsigned type, it is never less than zero.
> 
> Fix this by using an extra varibale called 'res', to store return value
> from sysfs_match_string() and assign that to ctlr->mode on the success
> path.
> 
> 
> [...]

Applied to 6.8/scsi-queue, thanks!

[1/1] scsi: fcoe: Fix unsigned comparison with zero in store_ctlr_mode()
      https://git.kernel.org/mkp/scsi/c/567a1e852e87
  

Patch

diff --git a/drivers/scsi/fcoe/fcoe_sysfs.c b/drivers/scsi/fcoe/fcoe_sysfs.c
index 408a806bf4c2..c64a085a7ee2 100644
--- a/drivers/scsi/fcoe/fcoe_sysfs.c
+++ b/drivers/scsi/fcoe/fcoe_sysfs.c
@@ -263,6 +263,7 @@  static ssize_t store_ctlr_mode(struct device *dev,
 			       const char *buf, size_t count)
 {
 	struct fcoe_ctlr_device *ctlr = dev_to_ctlr(dev);
+	int res;
 
 	if (count > FCOE_MAX_MODENAME_LEN)
 		return -EINVAL;
@@ -279,12 +280,13 @@  static ssize_t store_ctlr_mode(struct device *dev,
 			return -ENOTSUPP;
 		}
 
-		ctlr->mode = sysfs_match_string(fip_conn_type_names, buf);
-		if (ctlr->mode < 0 || ctlr->mode == FIP_CONN_TYPE_UNKNOWN) {
+		res = sysfs_match_string(fip_conn_type_names, buf);
+		if (res < 0 || res == FIP_CONN_TYPE_UNKNOWN) {
 			LIBFCOE_SYSFS_DBG(ctlr, "Unknown mode %s provided.\n",
 					  buf);
 			return -EINVAL;
 		}
+		ctlr->mode = res;
 
 		ctlr->f->set_fcoe_ctlr_mode(ctlr);
 		LIBFCOE_SYSFS_DBG(ctlr, "Mode changed to %s.\n", buf);