Re: [PATCH] Protect against overflow in dev_loss_tmo

[James Smart <james.smart@emulex.com>]

I don't ever expect to see large dev_loss_tmo values, but the patch is fine.

Acked-by: James Smart <james.smart@emulex.com>

-- james s


Hannes Reinecke wrote:
> The rport structure defines dev_loss_tmo as u32, which is
> later multiplied with HZ to get the actual timeout value.
> This might overflow for large dev_loss_tmo values. So we
> should be better using u64 as intermediate variables here
> to protect against overflow.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
>
> diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
> index 79660ee..9860322 100644
> --- a/drivers/scsi/scsi_transport_fc.c
> +++ b/drivers/scsi/scsi_transport_fc.c
> @@ -833,7 +833,7 @@ static ssize_t
>  store_fc_rport_dev_loss_tmo(struct device *dev, struct device_attribute *attr,
>  			    const char *buf, size_t count)
>  {
> -	int val;
> +	unsigned long val;
>  	struct fc_rport *rport = transport_class_to_rport(dev);
>  	struct Scsi_Host *shost = rport_to_shost(rport);
>  	struct fc_internal *i = to_fc_internal(shost->transportt);
> @@ -847,6 +847,12 @@ store_fc_rport_dev_loss_tmo(struct device *dev, struct device_attribute *attr,
>  		return -EINVAL;
>  
>  	/*
> +	 * Check for overflow; dev_loss_tmo is u32
> +	 */
> +	if (val > UINT_MAX)
> +		return -EINVAL;
> +
> +	/*
>  	 * If fast_io_fail is off we have to cap
>  	 * dev_loss_tmo at SCSI_DEVICE_BLOCK_MAX_TIMEOUT
>  	 */
> @@ -2852,7 +2858,7 @@ void
>  fc_remote_port_delete(struct fc_rport  *rport)
>  {
>  	struct Scsi_Host *shost = rport_to_shost(rport);
> -	int timeout = rport->dev_loss_tmo;
> +	unsigned long timeout = rport->dev_loss_tmo;
>  	unsigned long flags;
>  
>  	/*
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>