DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net/netvsc: fix event buffer leak on realloc failure
@ 2026-10-25  1:46 Chen Zhao
  2026-10-25  2:18 ` Chen Zhao
  2026-10-26 16:58 ` Stephen Hemminger
  0 siblings, 2 replies; 4+ messages in thread
From: Chen Zhao @ 2026-10-25  1:46 UTC (permalink / raw)
  To: weh; +Cc: thomas, stephen, dev, stable

rte_realloc() failure overwrites event_buf and leaks the old allocation.
Save the old pointer and free it on failure before resetting event_sz.

Fixes: 1f2766b7ee02 ("net/netvsc: resize event buffer as needed")
Cc: sthemmin@microsoft.com
Cc: stable@dpdk.org
Signed-off-by: Chen Zhao <ch3nzhao@gmail.com>
---
 .mailmap                     | 1 +
 drivers/net/netvsc/hn_rxtx.c | 8 ++++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/.mailmap b/.mailmap
index 6d9e262cfb..deae6514dd 100644
--- a/.mailmap
+++ b/.mailmap
@@ -267,6 +267,7 @@ Charles Myers <charles.myers@spirent.com>
 Charles Stoll <charles.stoll@intel.com>
 Chas Williams <3chas3@gmail.com> <chas3@att.com>
 Chas Williams <3chas3@gmail.com> <ciwillia@brocade.com>
+Chen Zhao <ch3nzhao@gmail.com>
 Chenbo Xia <chenbox@nvidia.com> <chenbo.xia@intel.com>
 Cheng Jiang <honest.jiang@foxmail.com> <cheng1.jiang@intel.com>
 Cheng Liu <liucheng11@huawei.com>
diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c
index 6f536610f2..4db6c933c9 100644
--- a/drivers/net/netvsc/hn_rxtx.c
+++ b/drivers/net/netvsc/hn_rxtx.c
@@ -1131,15 +1131,19 @@ uint32_t hn_process_events(struct hn_data *hv, uint16_t queue_id,
 
 		if (unlikely(ret == -ENOBUFS)) {
 			/* event buffer not large enough to read ring */
+			void *old_buf = rxq->event_buf;
 
 			PMD_DRV_LOG(DEBUG,
 				    "event buffer expansion (need %u)", len);
 			rxq->event_sz = len + len / 4;
-			rxq->event_buf = rte_realloc(rxq->event_buf, rxq->event_sz,
+			rxq->event_buf = rte_realloc(old_buf, rxq->event_sz,
 						     RTE_CACHE_LINE_SIZE);
 			if (rxq->event_buf)
 				goto retry;
-			/* out of memory, no more events now */
+			/*
+			 * out of memory, free the old buffer, no more events now
+			 */
+			rte_free(old_buf);
 			rxq->event_sz = 0;
 			break;
 		}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] net/netvsc: fix event buffer leak on realloc failure
  2026-10-25  1:46 [PATCH] net/netvsc: fix event buffer leak on realloc failure Chen Zhao
@ 2026-10-25  2:18 ` Chen Zhao
  2026-10-25 15:55   ` Stephen Hemminger
  2026-10-26 16:58 ` Stephen Hemminger
  1 sibling, 1 reply; 4+ messages in thread
From: Chen Zhao @ 2026-10-25  2:18 UTC (permalink / raw)
  To: weh; +Cc: thomas, stephen, dev, stable, dpdklab

Hi,

The AI review reports a double-free and a dangling event_buf:
https://mails.dpdk.org/archives/test-report/2026-October/1069492.html

Both findings are false positives:

1. On successful reallocation, goto retry skips rte_free(old_buf).
   The retry label is outside the ENOBUFS block. Each subsequent entry
   into that block reinitializes old_buf from the current event_buf,
   so a later allocation failure frees the current allocation, not an
   address retained from a previous successful reallocation.

2. On failure, rte_realloc() returns NULL without freeing the old buffer.
   That NULL return value has already been assigned to rxq->event_buf
   before rte_free(old_buf) is reached. Thus event_buf is already NULL;
   no additional NULL assignment is needed.

The patch preserves the existing reset-and-retry behavior while freeing
the old allocation on failure.

Thanks,
Chen

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] net/netvsc: fix event buffer leak on realloc failure
  2026-10-25  2:18 ` Chen Zhao
@ 2026-10-25 15:55   ` Stephen Hemminger
  0 siblings, 0 replies; 4+ messages in thread
From: Stephen Hemminger @ 2026-10-25 15:55 UTC (permalink / raw)
  To: Chen Zhao; +Cc: weh, thomas, dev, stable, dpdklab

On Sun, 25 Oct 2026 02:18:02 +0000
Chen Zhao <ch3nzhao@gmail.com> wrote:

> Hi,
> 
> The AI review reports a double-free and a dangling event_buf:
> https://mails.dpdk.org/archives/test-report/2026-October/1069492.html
> 
> Both findings are false positives:
> 
> 1. On successful reallocation, goto retry skips rte_free(old_buf).
>    The retry label is outside the ENOBUFS block. Each subsequent entry
>    into that block reinitializes old_buf from the current event_buf,
>    so a later allocation failure frees the current allocation, not an
>    address retained from a previous successful reallocation.
> 
> 2. On failure, rte_realloc() returns NULL without freeing the old buffer.
>    That NULL return value has already been assigned to rxq->event_buf
>    before rte_free(old_buf) is reached. Thus event_buf is already NULL;
>    no additional NULL assignment is needed.
> 
> The patch preserves the existing reset-and-retry behavior while freeing
> the old allocation on failure.
> 
> Thanks,
> Chen

The CI AI model is still using to weak a model and I never believe anything it says

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] net/netvsc: fix event buffer leak on realloc failure
  2026-10-25  1:46 [PATCH] net/netvsc: fix event buffer leak on realloc failure Chen Zhao
  2026-10-25  2:18 ` Chen Zhao
@ 2026-10-26 16:58 ` Stephen Hemminger
  1 sibling, 0 replies; 4+ messages in thread
From: Stephen Hemminger @ 2026-10-26 16:58 UTC (permalink / raw)
  To: Chen Zhao; +Cc: weh, thomas, dev, stable

On Sun, 25 Oct 2026 01:46:33 +0000
Chen Zhao <ch3nzhao@gmail.com> wrote:

> rte_realloc() failure overwrites event_buf and leaks the old allocation.
> Save the old pointer and free it on failure before resetting event_sz.
> 
> Fixes: 1f2766b7ee02 ("net/netvsc: resize event buffer as needed")
> Cc: sthemmin@microsoft.com
> Cc: stable@dpdk.org
> Signed-off-by: Chen Zhao <ch3nzhao@gmail.com>
> ---

AI review says "good but it could be better"

On Sun, 25 Oct 2026 01:46:33 +0000
Chen Zhao <ch3nzhao@gmail.com> wrote:

> rte_realloc() failure overwrites event_buf and leaks the old allocation.
> Save the old pointer and free it on failure before resetting event_sz.

The leak is real, but freeing the old buffer is the wrong recovery.
The old buffer is still valid and usable for smaller events; there is
no reason to throw it away. Also event_sz is updated before the
allocation succeeds.

Use a temporary and only commit on success:

		if (unlikely(ret == -ENOBUFS)) {
			/* event buffer not large enough to read ring */
			uint32_t new_sz = len + len / 4;
			void *new_buf;

			PMD_DRV_LOG(DEBUG,
				    "event buffer expansion (need %u)", len);
			new_buf = rte_realloc(rxq->event_buf, new_sz,
					      RTE_CACHE_LINE_SIZE);
			if (new_buf == NULL)
				break;	/* out of memory, no more events now */

			rxq->event_buf = new_buf;
			rxq->event_sz = new_sz;
			goto retry;
		}

Drop the Cc to sthemmin@microsoft.com, that address is dead.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-26 17:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-25  1:46 [PATCH] net/netvsc: fix event buffer leak on realloc failure Chen Zhao
2026-10-25  2:18 ` Chen Zhao
2026-10-25 15:55   ` Stephen Hemminger
2026-10-26 16:58 ` Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox