[PATCH v2 net-next 0/3] liquidio: avoid vm low memory crashes

[PATCH v2 net-next 0/3] liquidio: avoid vm low memory crashes

[Felix Manlunas]

From: Rick Farrington <ricardo.farrington@cavium.com>

This patchset addresses issues brought about by low memory conditions
in a VM.  These conditions were not seen when the driver was exercised
normally.  Rather, they were brought about through manual fault injection.
They are being included in the interest of hardening the driver against
unforeseen circumstances.

1. Fix GPF in octeon_init_droq(); zero the allocated block 'recv_buf_list'.
   This prevents a GPF trying to access an invalid 'recv_buf_list[i]' entry
   in octeon_droq_destroy_ring_buffers() if init didn't alloc all entries.
2. Don't dereference a NULL ptr in octeon_droq_destroy_ring_buffers().
3. For defensive programming, zero the allocated block 'oct->droq[0]' in
   octeon_setup_output_queues() and 'oct->instr_queue[0]' in    
   octeon_setup_instr_queues().

change log:
V1 -> V2:
1. Corrected syntax in 'Subject' lines; no functional or code changes.

Rick Farrington (3):
  liquidio: lowmem: init allocated memory to 0
  liquidio: lowmem: do not dereference null ptr
  liquidio: lowmem: init allocated memory to 0

 drivers/net/ethernet/cavium/liquidio/octeon_device.c | 8 ++++----
 drivers/net/ethernet/cavium/liquidio/octeon_droq.c   | 6 ++++--
 2 files changed, 8 insertions(+), 6 deletions(-)

-- 
2.9.0

[PATCH v2 net-next 1/3] liquidio: lowmem: init allocated memory to 0

[Felix Manlunas]

From: Rick Farrington <ricardo.farrington@cavium.com>

Fix GPF in octeon_init_droq(); zero the allocated block 'recv_buf_list'.
This prevents a GPF trying to access an invalid 'recv_buf_list[i]' entry
in octeon_droq_destroy_ring_buffers() if init didn't alloc all entries.

Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
Signed-off-by: Satanand Burla <satananda.burla@cavium.com>
Signed-off-by: Raghu Vatsavayi <raghu.vatsavayi@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
 drivers/net/ethernet/cavium/liquidio/octeon_droq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_droq.c b/drivers/net/ethernet/cavium/liquidio/octeon_droq.c
index 2e190de..6456683 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_droq.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_droq.c
@@ -275,12 +275,12 @@ int octeon_init_droq(struct octeon_device *oct,
 		droq->max_count);
 
 	droq->recv_buf_list = (struct octeon_recv_buffer *)
-			      vmalloc_node(droq->max_count *
+			      vzalloc_node(droq->max_count *
 						OCT_DROQ_RECVBUF_SIZE,
 						numa_node);
 	if (!droq->recv_buf_list)
 		droq->recv_buf_list = (struct octeon_recv_buffer *)
-				      vmalloc(droq->max_count *
+				      vzalloc(droq->max_count *
 						OCT_DROQ_RECVBUF_SIZE);
 	if (!droq->recv_buf_list) {
 		dev_err(&oct->pci_dev->dev, "Output queue recv buf list alloc failed\n");
-- 
2.9.0

[PATCH v2 net-next 2/3] liquidio: lowmem: do not dereference null ptr

[Felix Manlunas]

From: Rick Farrington <ricardo.farrington@cavium.com>

Don't dereference a NULL ptr in octeon_droq_destroy_ring_buffers().

Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
Signed-off-by: Satanand Burla <satananda.burla@cavium.com>
Signed-off-by: Raghu Vatsavayi <raghu.vatsavayi@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
 drivers/net/ethernet/cavium/liquidio/octeon_droq.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_droq.c b/drivers/net/ethernet/cavium/liquidio/octeon_droq.c
index 6456683..f7b5d68 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_droq.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_droq.c
@@ -145,6 +145,8 @@ octeon_droq_destroy_ring_buffers(struct octeon_device *oct,
 
 	for (i = 0; i < droq->max_count; i++) {
 		pg_info = &droq->recv_buf_list[i].pg_info;
+		if (!pg_info)
+			continue;
 
 		if (pg_info->dma)
 			lio_unmap_ring(oct->pci_dev,
-- 
2.9.0

[PATCH v2 net-next 3/3] liquidio: lowmem: init allocated memory to 0

[Felix Manlunas]

From: Rick Farrington <ricardo.farrington@cavium.com>

For defensive programming, zero the allocated block 'oct->droq[0]' in
octeon_setup_output_queues() and 'oct->instr_queue[0]' in
octeon_setup_instr_queues().

Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
Signed-off-by: Satanand Burla <satananda.burla@cavium.com>
Signed-off-by: Raghu Vatsavayi <raghu.vatsavayi@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
 drivers/net/ethernet/cavium/liquidio/octeon_device.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_device.c b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
index 623e28c..f10014f7 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_device.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
@@ -876,11 +876,11 @@ int octeon_setup_instr_queues(struct octeon_device *oct)
 
 	oct->num_iqs = 0;
 
-	oct->instr_queue[0] = vmalloc_node(sizeof(*oct->instr_queue[0]),
+	oct->instr_queue[0] = vzalloc_node(sizeof(*oct->instr_queue[0]),
 				numa_node);
 	if (!oct->instr_queue[0])
 		oct->instr_queue[0] =
-			vmalloc(sizeof(struct octeon_instr_queue));
+			vzalloc(sizeof(struct octeon_instr_queue));
 	if (!oct->instr_queue[0])
 		return 1;
 	memset(oct->instr_queue[0], 0, sizeof(struct octeon_instr_queue));
@@ -923,9 +923,9 @@ int octeon_setup_output_queues(struct octeon_device *oct)
 		desc_size = CFG_GET_DEF_RX_BUF_SIZE(CHIP_CONF(oct, cn23xx_vf));
 	}
 	oct->num_oqs = 0;
-	oct->droq[0] = vmalloc_node(sizeof(*oct->droq[0]), numa_node);
+	oct->droq[0] = vzalloc_node(sizeof(*oct->droq[0]), numa_node);
 	if (!oct->droq[0])
-		oct->droq[0] = vmalloc(sizeof(*oct->droq[0]));
+		oct->droq[0] = vzalloc(sizeof(*oct->droq[0]));
 	if (!oct->droq[0])
 		return 1;
 
-- 
2.9.0

Re: [PATCH v2 net-next 0/3] liquidio: avoid vm low memory crashes

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]

On Mon, Jul 17, 2017 at 05:49:20PM -0700, Felix Manlunas wrote:
> From: Rick Farrington <ricardo.farrington@cavium.com>
>
> This patchset addresses issues brought about by low memory conditions
> in a VM.  These conditions were not seen when the driver was exercised
> normally.  Rather, they were brought about through manual fault injection.
> They are being included in the interest of hardening the driver against
> unforeseen circumstances.
>
> 1. Fix GPF in octeon_init_droq(); zero the allocated block 'recv_buf_list'.
>    This prevents a GPF trying to access an invalid 'recv_buf_list[i]' entry
>    in octeon_droq_destroy_ring_buffers() if init didn't alloc all entries.
> 2. Don't dereference a NULL ptr in octeon_droq_destroy_ring_buffers().
> 3. For defensive programming, zero the allocated block 'oct->droq[0]' in
>    octeon_setup_output_queues() and 'oct->instr_queue[0]' in
>    octeon_setup_instr_queues().
>
> change log:
> V1 -> V2:
> 1. Corrected syntax in 'Subject' lines; no functional or code changes.
>
> Rick Farrington (3):
>   liquidio: lowmem: init allocated memory to 0
>   liquidio: lowmem: do not dereference null ptr
>   liquidio: lowmem: init allocated memory to 0

I'm feeling déjà vu here. We already discussed that zero allocated arrays
have nothing to do with low memory conditions. Why are you continuing to use
this misleading term here?

>
>  drivers/net/ethernet/cavium/liquidio/octeon_device.c | 8 ++++----
>  drivers/net/ethernet/cavium/liquidio/octeon_droq.c   | 6 ++++--
>  2 files changed, 8 insertions(+), 6 deletions(-)
>
> --
> 2.9.0
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH v2 net-next 0/3] liquidio: avoid vm low memory crashes

[Ricardo Farrington]

My apologies Leon - I did not infer that the subject line should have been changed from your previous correspondence.  I will correct it.

Rick

-----Original Message-----
From: Leon Romanovsky [mailto:leon@kernel.org] 
Sent: Monday, July 17, 2017 11:23 PM
To: Manlunas, Felix <Felix.Manlunas@cavium.com>
Cc: davem@davemloft.net; netdev@vger.kernel.org; Vatsavayi, Raghu <Raghu.Vatsavayi@cavium.com>; Chickles, Derek <Derek.Chickles@cavium.com>; Burla, Satananda <Satananda.Burla@cavium.com>; Ricardo Farrington <Ricardo.Farrington@cavium.com>
Subject: Re: [PATCH v2 net-next 0/3] liquidio: avoid vm low memory crashes

On Mon, Jul 17, 2017 at 05:49:20PM -0700, Felix Manlunas wrote:
> From: Rick Farrington <ricardo.farrington@cavium.com>
>
> This patchset addresses issues brought about by low memory conditions 
> in a VM.  These conditions were not seen when the driver was exercised 
> normally.  Rather, they were brought about through manual fault injection.
> They are being included in the interest of hardening the driver 
> against unforeseen circumstances.
>
> 1. Fix GPF in octeon_init_droq(); zero the allocated block 'recv_buf_list'.
>    This prevents a GPF trying to access an invalid 'recv_buf_list[i]' entry
>    in octeon_droq_destroy_ring_buffers() if init didn't alloc all entries.
> 2. Don't dereference a NULL ptr in octeon_droq_destroy_ring_buffers().
> 3. For defensive programming, zero the allocated block 'oct->droq[0]' in
>    octeon_setup_output_queues() and 'oct->instr_queue[0]' in
>    octeon_setup_instr_queues().
>
> change log:
> V1 -> V2:
> 1. Corrected syntax in 'Subject' lines; no functional or code changes.
>
> Rick Farrington (3):
>   liquidio: lowmem: init allocated memory to 0
>   liquidio: lowmem: do not dereference null ptr
>   liquidio: lowmem: init allocated memory to 0

I'm feeling déjà vu here. We already discussed that zero allocated arrays have nothing to do with low memory conditions. Why are you continuing to use this misleading term here?

>
>  drivers/net/ethernet/cavium/liquidio/octeon_device.c | 8 ++++----
>  drivers/net/ethernet/cavium/liquidio/octeon_droq.c   | 6 ++++--
>  2 files changed, 8 insertions(+), 6 deletions(-)
>
> --
> 2.9.0
>

Re: [PATCH v2 net-next 0/3] liquidio: avoid vm low memory crashes

[David Miller]

From: Felix Manlunas <felix.manlunas@cavium.com>
Date: Mon, 17 Jul 2017 17:49:20 -0700

> From: Rick Farrington <ricardo.farrington@cavium.com>
> 
> This patchset addresses issues brought about by low memory conditions
> in a VM.  These conditions were not seen when the driver was exercised
> normally.  Rather, they were brought about through manual fault injection.
> They are being included in the interest of hardening the driver against
> unforeseen circumstances.
> 
> 1. Fix GPF in octeon_init_droq(); zero the allocated block 'recv_buf_list'.
>    This prevents a GPF trying to access an invalid 'recv_buf_list[i]' entry
>    in octeon_droq_destroy_ring_buffers() if init didn't alloc all entries.
> 2. Don't dereference a NULL ptr in octeon_droq_destroy_ring_buffers().
> 3. For defensive programming, zero the allocated block 'oct->droq[0]' in
>    octeon_setup_output_queues() and 'oct->instr_queue[0]' in    
>    octeon_setup_instr_queues().
> 
> change log:
> V1 -> V2:
> 1. Corrected syntax in 'Subject' lines; no functional or code changes.

Series applied.  I removed the "[0]" in this commit log message.

Thanks.