|SUCCESS| examples/flow_filtering: fix null derefs and memory leaks

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/162794

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-01

git sha: Unknown (review of series.patch)
Author: Shani Peretz <shperetz at nvidia.com>
Subject: [PATCH] examples/flow_filtering: fix null derefs and memory leaks

This patch adds missing return statements after failed calloc NULL checks
and frees previously allocated memory on error paths to prevent resource
leaks.

> diff --git a/examples/flow_filtering/snippets/snippet_match_gre.c b/examples/flow_filtering/snippets/snippet_match_gre.c
> index 477ec59451..f1638f38c5 100644
> --- a/examples/flow_filtering/snippets/snippet_match_gre.c
> +++ b/examples/flow_filtering/snippets/snippet_match_gre.c

[ ... ]

> @@ -21,8 +21,10 @@ snippet_match_gre_create_actions(__rte_unused uint16_t port_id, struct rte_flow_
>  {
>  	/* Create one action that moves the packet to the selected queue. */
>  	struct rte_flow_action_queue *queue = calloc(1, sizeof(struct rte_flow_action_queue));
> -	if (queue == NULL)
> +	if (queue == NULL) {
>  		fprintf(stderr, "Failed to allocate memory for queue\n");
> +		return;
> +	}

Does the code still free the queue structure on the normal path? The patch
only shows the error handling change, but does not show where queue gets
freed after successful use. Does snippet_match_gre_create_actions() or its
caller ensure this allocation is eventually freed?

>  	/* Set the selected queue. */
>  	queue->index = 1;

[ ... ]

> @@ -40,12 +42,17 @@ snippet_match_gre_create_patterns(struct rte_flow_item *pattern)
>  	struct rte_flow_item_gre_opt *gre_opt_spec;
>  
>  	gre_spec = calloc(1, sizeof(struct rte_flow_item_gre));
> -	if (gre_spec == NULL)
> +	if (gre_spec == NULL) {
>  		fprintf(stderr, "Failed to allocate memory for gre_spec\n");
> +		return;
> +	}
>  
>  	gre_opt_spec = calloc(1, sizeof(struct rte_flow_item_gre_opt));
> -	if (gre_opt_spec == NULL)
> +	if (gre_opt_spec == NULL) {
>  		fprintf(stderr, "Failed to allocate memory for gre_opt_spec\n");
> +		free(gre_spec);
> +		return;
> +	}

[ ... ]

> diff --git a/examples/flow_filtering/snippets/snippet_match_integrity_flags.c b/examples/flow_filtering/snippets/snippet_match_integrity_flags.c
> index 0f05c902b9..3d37b5f2a5 100644
> --- a/examples/flow_filtering/snippets/snippet_match_integrity_flags.c
> +++ b/examples/flow_filtering/snippets/snippet_match_integrity_flags.c

[ ... ]

> @@ -38,16 +40,21 @@ snippet_match_integrity_flags_create_patterns(struct rte_flow_item *pattern)
>  	struct rte_flow_item_integrity *integrity_mask;
>  
>  	integrity_spec = calloc(1, sizeof(struct rte_flow_item_integrity));
> -	if (integrity_spec == NULL)
> +	if (integrity_spec == NULL) {
>  		fprintf(stderr, "Failed to allocate memory for integrity_spec\n");
> +		return;
> +	}
>  
>  	integrity_spec->level = 0;
>  	integrity_spec->l3_ok = 1;
>  	integrity_spec->ipv4_csum_ok = 1;
>  
>  	integrity_mask = calloc(1, sizeof(struct rte_flow_item_integrity));
> -	if (integrity_mask == NULL)
> +	if (integrity_mask == NULL) {
>  		fprintf(stderr, "Failed to allocate memory for integrity_mask\n");
> +		free(integrity_spec);
> +		return;
> +	}
>  
>  	integrity_mask->level = 0;
>  	integrity_mask->l3_ok = 1;
> @@ -55,12 +62,21 @@ snippet_match_integrity_flags_create_patterns(struct rte_flow_item *pattern)
>  	integrity_mask->ipv4_csum_ok = 1;
>  
>  	ip_spec = calloc(1, sizeof(struct rte_flow_item_ipv4));
> -	if (ip_spec == NULL)
> +	if (ip_spec == NULL) {
>  		fprintf(stderr, "Failed to allocate memory for ip_spec\n");
> +		free(integrity_spec);
> +		free(integrity_mask);
> +		return;
> +	}
>  
>  	ip_mask = calloc(1, sizeof(struct rte_flow_item_ipv4));
> -	if (ip_mask == NULL)
> +	if (ip_mask == NULL) {
>  		fprintf(stderr, "Failed to allocate memory for ip_mask\n");
> +		free(integrity_spec);
> +		free(integrity_mask);
> +		free(ip_spec);
> +		return;
> +	}

[ ... ]

> diff --git a/examples/flow_filtering/snippets/snippet_match_mpls.c b/examples/flow_filtering/snippets/snippet_match_mpls.c
> index 494a2d873d..0a54586c98 100644
> --- a/examples/flow_filtering/snippets/snippet_match_mpls.c
> +++ b/examples/flow_filtering/snippets/snippet_match_mpls.c
> @@ -25,8 +25,10 @@ snippet_mpls_create_actions(__rte_unused uint16_t port_id, struct rte_flow_actio
>  	struct rte_flow_action_queue *queue;
>  
>  	queue = calloc(1, sizeof(struct rte_flow_item_ipv4));

Does this code allocate the wrong size? The variable queue is of type
rte_flow_action_queue but the calloc uses sizeof(struct
rte_flow_item_ipv4). Does calloc use the correct size for queue?

> -	if (queue == NULL)
> +	if (queue == NULL) {
>  		fprintf(stderr, "Failed to allocate memory for queue\n");
> +		return;
> +	}

[ ... ]

> @@ -43,8 +45,10 @@ snippet_mpls_create_patterns(struct rte_flow_item *pattern)
>  	struct rte_flow_item_mpls *mpls_item;
>  
>  	mpls_item = calloc(1, sizeof(struct rte_flow_item_ipv4));

Does this code also allocate the wrong size? The variable mpls_item is of
type rte_flow_item_mpls but the calloc uses sizeof(struct
rte_flow_item_ipv4). Does calloc use the correct size for mpls_item?

> -	if (mpls_item == NULL)
> +	if (mpls_item == NULL) {
>  		fprintf(stderr, "Failed to allocate memory for mpls_item\n");
> +		return;
> +	}

[ ... ]

> diff --git a/examples/flow_filtering/snippets/snippet_modify_ecn.c b/examples/flow_filtering/snippets/snippet_modify_ecn.c
> index 74bc708036..563a394dc6 100644
> --- a/examples/flow_filtering/snippets/snippet_modify_ecn.c
> +++ b/examples/flow_filtering/snippets/snippet_modify_ecn.c
> @@ -22,13 +22,18 @@ snippet_match_modify_ecn_create_actions(__rte_unused uint16_t port_id,
>  {
>  	/* Create one action that moves the packet to the selected queue. */
>  	struct rte_flow_action_queue *queue = calloc(1, sizeof(struct rte_flow_action_queue));
> -	if (queue == NULL)
> +	if (queue == NULL) {
>  		fprintf(stderr, "Failed to allocate memory for queue\n");
> +		return;
> +	}
>  
>  	struct rte_flow_action_modify_field *modify_field =
>  					calloc(1, sizeof(struct rte_flow_action_modify_field));
> -	if (modify_field == NULL)
> +	if (modify_field == NULL) {
>  		fprintf(stderr, "Failed to allocate memory for modify_field\n");
> +		free(queue);
> +		return;
> +	}

The error paths now correctly free queue, but does the normal exit path
from snippet_match_modify_ecn_create_actions() also leak both queue and
modify_field? Where are these allocations freed on success?

[ ... ]