[SRU,X] scsi_dh_alua: uninitialized variable in alua_rtpg()

Message ID CAAZtj82nbgE3QM5sPPUnL_tD__n_n_QN5o-p5CughSTFY-WFdA@mail.gmail.com
State New
Headers show
Series
  • [SRU,X] scsi_dh_alua: uninitialized variable in alua_rtpg()
Related show

Commit Message

Dragan Stancevic Nov. 29, 2017, 11:19 p.m.
From: Dan Carpenter <dan.carpenter@oracle.com>

BugLink: https://bugs.launchpad.net/bugs/1720228

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>
Reviewed-by: Hannes Reinicke <hare@suse.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: James Bottomley <jejb@linux.vnet.ibm.com>
(cherry picked from commit a4bd85203190990ad808abbd4a5dc848a950002c)
Signed-off-by: Dragan Stancevic <dragan.stancevic@canonical.com>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 1 +
 1 file changed, 1 insertion(+)

        if (retval) {

Comments

Colin King Nov. 30, 2017, 1:43 p.m. | #1
On 29/11/17 23:19, Dragan Stancevic wrote:
> From: Dan Carpenter <dan.carpenter@oracle.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1720228
> 
> 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>
> Reviewed-by: Hannes Reinicke <hare@suse.de>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> Signed-off-by: James Bottomley <jejb@linux.vnet.ibm.com>
> (cherry picked from commit a4bd85203190990ad808abbd4a5dc848a950002c)
> Signed-off-by: Dragan Stancevic <dragan.stancevic@canonical.com>
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> 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) {
> 
Clean upstream cherry pick, looks good.

Acked-by: Colin Ian King <colin.king@canonical.com>
Dragan Stancevic Dec. 11, 2017, 3:11 p.m. | #2
Has this gone in? I just did a git pull but don't see it in the tree.

I'm asking because I have a user that has been waiting on this for a
few months. If it hasn't can someone else also take a look at it so it
can move forward?

Thank you in advance

On Thu, Nov 30, 2017 at 7:43 AM, Colin Ian King
<colin.king@canonical.com> wrote:
> On 29/11/17 23:19, Dragan Stancevic wrote:
>> From: Dan Carpenter <dan.carpenter@oracle.com>
>>
>> BugLink: https://bugs.launchpad.net/bugs/1720228
>>
>> 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>
>> Reviewed-by: Hannes Reinicke <hare@suse.de>
>> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
>> Signed-off-by: James Bottomley <jejb@linux.vnet.ibm.com>
>> (cherry picked from commit a4bd85203190990ad808abbd4a5dc848a950002c)
>> Signed-off-by: Dragan Stancevic <dragan.stancevic@canonical.com>
>> ---
>>  drivers/scsi/device_handler/scsi_dh_alua.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> 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) {
>>
> Clean upstream cherry pick, looks good.
>
> Acked-by: Colin Ian King <colin.king@canonical.com>
Kleber Souza Dec. 12, 2017, 2:28 p.m. | #3
On 12/11/17 16:11, Dragan Stancevic wrote:
> Has this gone in? I just did a git pull but don't see it in the tree.
> 
> I'm asking because I have a user that has been waiting on this for a
> few months. If it hasn't can someone else also take a look at it so it
> can move forward?
> 

Hi Dragan,

The patch has got only one ACK on the mailing-list, it needs two ACKs
before we apply it to the master-next branch of the corresponding
series. When that's done, it will be picked up for the next SRU cycle.

However, the Stable Team has decided that the last SRU cycle of the
year, which is starting this week, will be focused on security (CVE)
fixes only. So your patch will be applied only for the first SRU cycle
of next year.


Regards,
Kleber

> Thank you in advance
> 
> On Thu, Nov 30, 2017 at 7:43 AM, Colin Ian King
> <colin.king@canonical.com> wrote:
>> On 29/11/17 23:19, Dragan Stancevic wrote:
>>> From: Dan Carpenter <dan.carpenter@oracle.com>
>>>
>>> BugLink: https://bugs.launchpad.net/bugs/1720228
>>>
>>> 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>
>>> Reviewed-by: Hannes Reinicke <hare@suse.de>
>>> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
>>> Signed-off-by: James Bottomley <jejb@linux.vnet.ibm.com>
>>> (cherry picked from commit a4bd85203190990ad808abbd4a5dc848a950002c)
>>> Signed-off-by: Dragan Stancevic <dragan.stancevic@canonical.com>
>>> ---
>>>  drivers/scsi/device_handler/scsi_dh_alua.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> 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) {
>>>
>> Clean upstream cherry pick, looks good.
>>
>> Acked-by: Colin Ian King <colin.king@canonical.com>
>
Kleber Souza Dec. 12, 2017, 2:31 p.m. | #4
On 11/30/17 00:19, Dragan Stancevic wrote:
> From: Dan Carpenter <dan.carpenter@oracle.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1720228
> 
> 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>
> Reviewed-by: Hannes Reinicke <hare@suse.de>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> Signed-off-by: James Bottomley <jejb@linux.vnet.ibm.com>
> (cherry picked from commit a4bd85203190990ad808abbd4a5dc848a950002c)
> Signed-off-by: Dragan Stancevic <dragan.stancevic@canonical.com>

Simple fix, clean cherry-pick.

Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>

> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> 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) {
>
Dragan Stancevic Dec. 12, 2017, 3:39 p.m. | #5
Got it, thanks

On Tue, Dec 12, 2017 at 8:28 AM, Kleber Souza
<kleber.souza@canonical.com> wrote:
> On 12/11/17 16:11, Dragan Stancevic wrote:
>> Has this gone in? I just did a git pull but don't see it in the tree.
>>
>> I'm asking because I have a user that has been waiting on this for a
>> few months. If it hasn't can someone else also take a look at it so it
>> can move forward?
>>
>
> Hi Dragan,
>
> The patch has got only one ACK on the mailing-list, it needs two ACKs
> before we apply it to the master-next branch of the corresponding
> series. When that's done, it will be picked up for the next SRU cycle.
>
> However, the Stable Team has decided that the last SRU cycle of the
> year, which is starting this week, will be focused on security (CVE)
> fixes only. So your patch will be applied only for the first SRU cycle
> of next year.
>
>
> Regards,
> Kleber
>
>> Thank you in advance
>>
>> On Thu, Nov 30, 2017 at 7:43 AM, Colin Ian King
>> <colin.king@canonical.com> wrote:
>>> On 29/11/17 23:19, Dragan Stancevic wrote:
>>>> From: Dan Carpenter <dan.carpenter@oracle.com>
>>>>
>>>> BugLink: https://bugs.launchpad.net/bugs/1720228
>>>>
>>>> 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>
>>>> Reviewed-by: Hannes Reinicke <hare@suse.de>
>>>> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
>>>> Signed-off-by: James Bottomley <jejb@linux.vnet.ibm.com>
>>>> (cherry picked from commit a4bd85203190990ad808abbd4a5dc848a950002c)
>>>> Signed-off-by: Dragan Stancevic <dragan.stancevic@canonical.com>
>>>> ---
>>>>  drivers/scsi/device_handler/scsi_dh_alua.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> 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) {
>>>>
>>> Clean upstream cherry pick, looks good.
>>>
>>> Acked-by: Colin Ian King <colin.king@canonical.com>
>>

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