|SUCCESS| ethdev: fix pointer check in GENEVE and RAW flow copy

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-13

git sha: 20260507112012.119140-1-lyulin.2003 at mail.ru
Author: Denis Lyulin <lyulin.2003 at mail.ru>
Subject: ethdev: fix pointer check in GENEVE and RAW flow copy

This patch adds NULL pointer checks in rte_flow_conv_item_spec() for the
GENEVE_OPT and RAW item types to prevent dereferencing spec when it is
NULL while copying last or mask values.

> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> index fe8f43caff..7cf585e6f5 100644
> --- a/lib/ethdev/rte_flow.c
> +++ b/lib/ethdev/rte_flow.c

[ ... ]

> @@ -672,13 +672,17 @@ rte_flow_conv_item_spec(void *buf, const size_t size,
>  			   }),
>  			   size > sizeof(*dst.raw) ? sizeof(*dst.raw) : size);
>  		off = sizeof(*dst.raw);
> -		if (type == RTE_FLOW_CONV_ITEM_SPEC ||
> -		    (type == RTE_FLOW_CONV_ITEM_MASK &&
> -		     ((spec.raw->length & mask.raw->length) >=
> -		      (last.raw->length & mask.raw->length))))
> -			tmp = spec.raw->length & mask.raw->length;
> -		else
> -			tmp = last.raw->length & mask.raw->length;
> +		if (spec.raw && last.raw) {

Does this code need to check mask.raw as well? The original code
references mask.raw->length in the nested condition, which could also be
NULL according to the commit message.

> +			if (type == RTE_FLOW_CONV_ITEM_SPEC ||
> +			    (type == RTE_FLOW_CONV_ITEM_MASK &&
> +			     ((spec.raw->length & mask.raw->length) >=
> +			      (last.raw->length & mask.raw->length))))
> +				tmp = spec.raw->length & mask.raw->length;
> +			else
> +				tmp = last.raw->length & mask.raw->length;
> +		} else {
> +			tmp = 0;
> +		}

Would it be clearer to initialize tmp = 0 at declaration and skip this
else block entirely?

>  		if (tmp) {
>  			off = RTE_ALIGN_CEIL(off, sizeof(*dst.raw->pattern));
>  			if (size >= off + tmp) {

[ ... ]

> @@ -696,8 +700,8 @@ rte_flow_conv_item_spec(void *buf, const size_t size,
>  		spec.geneve_opt = item->spec;
>  		src.geneve_opt = data;
>  		dst.geneve_opt = buf;
> -		tmp = spec.geneve_opt->option_len << 2;
> -		if (size > 0 && src.geneve_opt->data) {
> +		tmp = spec.geneve_opt ? (spec.geneve_opt->option_len << 2) : 0;
> +		if (size > 0 && tmp > 0 && src.geneve_opt->data) {

Does this condition properly handle the case when type is
RTE_FLOW_CONV_ITEM_LAST or RTE_FLOW_CONV_ITEM_MASK? The calculation of
tmp should potentially use last.geneve_opt or mask.geneve_opt depending
on the conversion type, similar to the RAW item handling above.

>  			deep_src = (void *)((uintptr_t)(dst.geneve_opt + 1));
>  			dst.geneve_opt->data = rte_memcpy(deep_src,
>  							  src.geneve_opt->data,