[PATCH AUTOSEL 5.9 15/23] scsi: storvsc: Validate length of incoming packet in storvsc_on_channel_callback()

[PATCH AUTOSEL 5.9 15/23] scsi: storvsc: Validate length of incoming packet in storvsc_on_channel_callback()

[Sasha Levin]

From: "Andrea Parri (Microsoft)" <parri.andrea@gmail.com>

[ Upstream commit 3b8c72d076c42bf27284cda7b2b2b522810686f8 ]

Check that the packet is of the expected size at least, don't copy data
past the packet.

Link: https://lore.kernel.org/r/20201118145348.109879-1-parri.andrea@gmail.com
Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org
Reported-by: Saruhan Karademir <skarade@microsoft.com>
Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/scsi/storvsc_drv.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 8f5f5dc863a4a..6779ee4edfee3 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1246,6 +1246,11 @@ static void storvsc_on_channel_callback(void *context)
 		request = (struct storvsc_cmd_request *)
 			((unsigned long)desc->trans_id);
 
+		if (hv_pkt_datalen(desc) < sizeof(struct vstor_packet) - vmscsi_size_delta) {
+			dev_err(&device->device, "Invalid packet len\n");
+			continue;
+		}
+
 		if (request == &stor_device->init_request ||
 		    request == &stor_device->reset_request) {
 			memcpy(&request->vstor_packet, packet,
-- 
2.27.0

[PATCH AUTOSEL 5.9 16/23] scsi: mpt3sas: Increase IOCInit request timeout to 30s

[Sasha Levin]

From: Sreekanth Reddy <sreekanth.reddy@broadcom.com>

[ Upstream commit 85dad327d9b58b4c9ce08189a2707167de392d23 ]

Currently the IOCInit request message timeout is set to 10s. This is not
sufficient in some scenarios such as during HBA FW downgrade operations.

Increase the IOCInit request timeout to 30s.

Link: https://lore.kernel.org/r/20201130082733.26120-1-sreekanth.reddy@broadcom.com
Signed-off-by: Sreekanth Reddy <sreekanth.reddy@broadcom.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 87d05c1950870..d62c1aaf53fbd 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -6441,7 +6441,7 @@ _base_send_ioc_init(struct MPT3SAS_ADAPTER *ioc)
 
 	r = _base_handshake_req_reply_wait(ioc,
 	    sizeof(Mpi2IOCInitRequest_t), (u32 *)&mpi_request,
-	    sizeof(Mpi2IOCInitReply_t), (u16 *)&mpi_reply, 10);
+	    sizeof(Mpi2IOCInitReply_t), (u16 *)&mpi_reply, 30);
 
 	if (r != 0) {
 		ioc_err(ioc, "%s: handshake failed (r=%d)\n", __func__, r);
-- 
2.27.0

Re: [PATCH AUTOSEL 5.9 15/23] scsi: storvsc: Validate length of incoming packet in storvsc_on_channel_callback()

[Andrea Parri]

Hi Sasha,

On Sat, Dec 12, 2020 at 11:07:56AM -0500, Sasha Levin wrote:
> From: "Andrea Parri (Microsoft)" <parri.andrea@gmail.com>
> 
> [ Upstream commit 3b8c72d076c42bf27284cda7b2b2b522810686f8 ]

FYI, we found that this commit introduced a regression and posted a
revert:

  https://lkml.kernel.org/r/20201211131404.21359-1-parri.andrea@gmail.com

Same comment for the AUTOSEL 5.4, 4.19 and 4.14 you've just posted.

  Andrea


> 
> Check that the packet is of the expected size at least, don't copy data
> past the packet.
> 
> Link: https://lore.kernel.org/r/20201118145348.109879-1-parri.andrea@gmail.com
> Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
> Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
> Cc: linux-scsi@vger.kernel.org
> Reported-by: Saruhan Karademir <skarade@microsoft.com>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  drivers/scsi/storvsc_drv.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 8f5f5dc863a4a..6779ee4edfee3 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1246,6 +1246,11 @@ static void storvsc_on_channel_callback(void *context)
>  		request = (struct storvsc_cmd_request *)
>  			((unsigned long)desc->trans_id);
>  
> +		if (hv_pkt_datalen(desc) < sizeof(struct vstor_packet) - vmscsi_size_delta) {
> +			dev_err(&device->device, "Invalid packet len\n");
> +			continue;
> +		}
> +
>  		if (request == &stor_device->init_request ||
>  		    request == &stor_device->reset_request) {
>  			memcpy(&request->vstor_packet, packet,
> -- 
> 2.27.0
> 

Re: [PATCH AUTOSEL 5.9 15/23] scsi: storvsc: Validate length of incoming packet in storvsc_on_channel_callback()

[Sasha Levin]

On Sat, Dec 12, 2020 at 07:09:01PM +0100, Andrea Parri wrote:
>Hi Sasha,
>
>On Sat, Dec 12, 2020 at 11:07:56AM -0500, Sasha Levin wrote:
>> From: "Andrea Parri (Microsoft)" <parri.andrea@gmail.com>
>>
>> [ Upstream commit 3b8c72d076c42bf27284cda7b2b2b522810686f8 ]
>
>FYI, we found that this commit introduced a regression and posted a
>revert:
>
>  https://lkml.kernel.org/r/20201211131404.21359-1-parri.andrea@gmail.com
>
>Same comment for the AUTOSEL 5.4, 4.19 and 4.14 you've just posted.

I'll drop those, thanks!

-- 
Thanks,
Sasha

Re: [PATCH AUTOSEL 5.9 15/23] scsi: storvsc: Validate length of incoming packet in storvsc_on_channel_callback()

[Dan Carpenter]

On Sat, Dec 12, 2020 at 07:09:01PM +0100, Andrea Parri wrote:
> Hi Sasha,
> 
> On Sat, Dec 12, 2020 at 11:07:56AM -0500, Sasha Levin wrote:
> > From: "Andrea Parri (Microsoft)" <parri.andrea@gmail.com>
> > 
> > [ Upstream commit 3b8c72d076c42bf27284cda7b2b2b522810686f8 ]
> 
> FYI, we found that this commit introduced a regression and posted a
> revert:
> 
>   https://lkml.kernel.org/r/20201211131404.21359-1-parri.andrea@gmail.com
> 
> Same comment for the AUTOSEL 5.4, 4.19 and 4.14 you've just posted.
> 

Konstantin, is there anyway we could make searching lore.kernel.org
search all the mailing lists?  Right now we can only search one mailing
list at a time.

Part of the stable process should be to search lore.kernel.org for
Fixes: 3b8c72d076c4 ("scsi: storvsc: Validate length of... ")

But, unfortunately, git revert sets people up for failure by not
including a fixes tag so we'd also have to search for:
This reverts commit 3b8c72d076c42bf27284cda7b2b2b522810686f8.

regards,
dan carpenter

Re: [PATCH AUTOSEL 5.9 15/23] scsi: storvsc: Validate length of incoming packet in storvsc_on_channel_callback()

[Konstantin Ryabitsev]

On Mon, Dec 14, 2020 at 02:07:11PM +0300, Dan Carpenter wrote:
> On Sat, Dec 12, 2020 at 07:09:01PM +0100, Andrea Parri wrote:
> > Hi Sasha,
> > 
> > On Sat, Dec 12, 2020 at 11:07:56AM -0500, Sasha Levin wrote:
> > > From: "Andrea Parri (Microsoft)" <parri.andrea@gmail.com>
> > > 
> > > [ Upstream commit 3b8c72d076c42bf27284cda7b2b2b522810686f8 ]
> > 
> > FYI, we found that this commit introduced a regression and posted a
> > revert:
> > 
> >   https://lkml.kernel.org/r/20201211131404.21359-1-parri.andrea@gmail.com
> > 
> > Same comment for the AUTOSEL 5.4, 4.19 and 4.14 you've just posted.
> > 
> 
> Konstantin, is there anyway we could make searching lore.kernel.org
> search all the mailing lists?  Right now we can only search one mailing
> list at a time.

This functionality is coming in the next version of public-inbox and 
should be available on lore.kernel.org within the next little while.

https://lore.kernel.org/workflows/20201201184814.GA32272@dcvr/

-K

Re: [PATCH AUTOSEL 5.9 15/23] scsi: storvsc: Validate length of incoming packet in storvsc_on_channel_callback()

[Sasha Levin]

On Mon, Dec 14, 2020 at 08:06:25AM -0500, Konstantin Ryabitsev wrote:
>On Mon, Dec 14, 2020 at 02:07:11PM +0300, Dan Carpenter wrote:
>> On Sat, Dec 12, 2020 at 07:09:01PM +0100, Andrea Parri wrote:
>> > Hi Sasha,
>> >
>> > On Sat, Dec 12, 2020 at 11:07:56AM -0500, Sasha Levin wrote:
>> > > From: "Andrea Parri (Microsoft)" <parri.andrea@gmail.com>
>> > >
>> > > [ Upstream commit 3b8c72d076c42bf27284cda7b2b2b522810686f8 ]
>> >
>> > FYI, we found that this commit introduced a regression and posted a
>> > revert:
>> >
>> >   https://lkml.kernel.org/r/20201211131404.21359-1-parri.andrea@gmail.com
>> >
>> > Same comment for the AUTOSEL 5.4, 4.19 and 4.14 you've just posted.
>> >
>>
>> Konstantin, is there anyway we could make searching lore.kernel.org
>> search all the mailing lists?  Right now we can only search one mailing
>> list at a time.
>
>This functionality is coming in the next version of public-inbox and
>should be available on lore.kernel.org within the next little while.

That's a good idea Dan; I had something running on linux-next, but I
guess it's not enough and cases such as these sneak in.

I wrote a script to do what you've suggested by simply cloning the repos
on erol.kernel.org locally and then doing a simple search for the
"Fixes:" and revert patterns.

-- 
Thanks,
Sasha