diff mbox

[v2] scsi_dh_alua: uninitialized variable in alua_rtpg()

Message ID 20160414182034.GA32469@mwanda (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Dan Carpenter April 14, 2016, 6:20 p.m. UTC
It's possible to use "err" without initializing it.  If it happens to be
a 2 which is SCSI_DH_RETRY then that could cause a bug.  Bart Van Assche
pointed out that we should probably re-initialize it for every iteration
through the retry loop.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: The first version just initialized it at the start of the function.

--
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

Comments

Bart Van Assche April 14, 2016, 6:55 p.m. UTC | #1
On 04/14/2016 11:20 AM, Dan Carpenter wrote:
> It's possible to use "err" without initializing it.  If it happens to be
> a 2 which is SCSI_DH_RETRY then that could cause a bug.  Bart Van Assche
> pointed out that we should probably re-initialize it for every iteration
> through the retry loop.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: The first version just initialized it at the start of the function.
>
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 8eaed05..a655cf2 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -532,6 +532,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
>   		return SCSI_DH_DEV_TEMP_BUSY;
>
>    retry:
> +	err = 0;
>   	retval = submit_rtpg(sdev, buff, bufflen, &sense_hdr, pg->flags);
>
>   	if (retval) {

Although I would have preferred that that initialization would have been 
closer to the other 'err' assignments this patch looks fine to me. If 
this patch does not get integrated in kernel v4.6 a "Cc: stable" tag 
will be needed.

Bart.
--
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
Hannes Reinecke April 15, 2016, 5:59 a.m. UTC | #2
On 04/14/2016 08:20 PM, Dan Carpenter wrote:
> It's possible to use "err" without initializing it.  If it happens to be
> a 2 which is SCSI_DH_RETRY then that could cause a bug.  Bart Van Assche
> pointed out that we should probably re-initialize it for every iteration
> through the retry loop.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: The first version just initialized it at the start of the function.
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 8eaed05..a655cf2 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -532,6 +532,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
>  		return SCSI_DH_DEV_TEMP_BUSY;
>  
>   retry:
> +	err = 0;
>  	retval = submit_rtpg(sdev, buff, bufflen, &sense_hdr, pg->flags);
>  
>  	if (retval) {
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
Martin K. Petersen April 15, 2016, 8:26 p.m. UTC | #3
>>>>> "Dan" == Dan Carpenter <dan.carpenter@oracle.com> writes:

Dan> It's possible to use "err" without initializing it.  If it happens
Dan> to be a 2 which is SCSI_DH_RETRY then that could cause a bug.  Bart
Dan> Van Assche pointed out that we should probably re-initialize it for
Dan> every iteration through the retry loop.

Applied to 4.6/scsi-fixes.
diff mbox

Patch

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 8eaed05..a655cf2 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -532,6 +532,7 @@  static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 		return SCSI_DH_DEV_TEMP_BUSY;
 
  retry:
+	err = 0;
 	retval = submit_rtpg(sdev, buff, bufflen, &sense_hdr, pg->flags);
 
 	if (retval) {