Re: [PATCH v3 1/5] ethdev: fix flow conversion with null buffer

[Stephen Hemminger <stephen@networkplumber.org>]

On Sat, 24 Oct 2026 14:50:28 +0100
Anatoly Burakov <anatoly.burakov@intel.com> wrote:

> rte_flow_conv_pattern() and rte_flow_conv_actions() document that the
> destination buffer may be NULL when the size is zero, which is how
> callers query the number of bytes required before allocating. Both
> functions nonetheless walked the destination with ++dst in the first
> pass and dst -= num before the second, regardless of size.
> 
> Pointer arithmetic on a NULL pointer is undefined behavior, and is
> reported by UBSan as "applying non-zero offset to null pointer" for
> every RTE_FLOW_CONV_OP_PATTERN and RTE_FLOW_CONV_OP_ACTIONS size query.
> 
> Keep the destination pointer fixed and index it only inside the
> size-guarded writes, which are never reached when size is zero.
> 
> Fixes: 063911ee1df4 ("ethdev: add flow API object converter")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---

Manually run AI review sees some problems.

Subject: Re: [PATCH v3 0/5] ethdev: flow conversion fixes

Series applies cleanly to main and each commit builds with
-Dwerror=true. Descriptor tables now cover every item type through
COMPARE and every action type through JUMP_TO_TABLE_INDEX.

One correctness problem in 3/5, plus a pre-existing out-of-bounds
write on the same code path that should be fixed while here.


[PATCH v3 3/5] ethdev: fix missing flow action descriptors

Error: SHARED is now accepted but its handle is not preserved.

The new entry is MK_FLOW_ACTION(SHARED, 0) with a comment saying the
handle is stored in conf as for INDIRECT. rte_flow_conv_actions()
only special-cases RTE_FLOW_ACTION_TYPE_INDIRECT. SHARED with a
non-NULL conf falls into the generic branch, rte_flow_conv_copy()
copies zero bytes (desc size 0), and dst[i].conf is set to
data + off, a pointer into the output buffer instead of the handle.

Before this patch SHARED was rejected with ENOTSUP. After it,
rte_flow_conv() and rte_flow_copy() succeed and return a corrupted
action. Handle it the same way as INDIRECT:

-		if (src->type == RTE_FLOW_ACTION_TYPE_INDIRECT) {
+		if (src->type == RTE_FLOW_ACTION_TYPE_INDIRECT ||
+		    src->type == RTE_FLOW_ACTION_TYPE_SHARED) {

Either that, or leave SHARED out of the table since it is deprecated;
but then the completeness test in 4/5 needs an exception for it.

Please add a SHARED action with a non-NULL conf to
test_flow_conv_actions() and check out[i].conf == actions[i].conf.
That would have caught this.


[PATCH v3 1/5] ethdev: fix flow conversion with null buffer

Warning: INDIRECT conf write is not bounded by the buffer size.

Pre-existing, but this patch rewrites the line. In the second pass:

	if (size)
		dst[i].conf = src->conf;

Every other write into dst[] is guarded by size covering the entry.
With actions { QUEUE, INDIRECT, END } and size equal to
sizeof(struct rte_flow_action), the first pass writes only dst[0],
then the second pass writes dst[1].conf past the end of the caller's
buffer. Suggest:

-			if (size)
+			if (size >= (i + 1) * sizeof(*dst))
 				dst[i].conf = src->conf;

This also covers SHARED once 3/5 routes it through the same branch.


[PATCH v3 5/5] ethdev: add flow item structure size query

Info: the NULL name checks added to rte_flow_conv_name() and
rte_flow_conv_item_default_mask() are hardening for the strlcpy(NULL)
case described in 2/5 and 3/5. With those two patches applied there
are no gaps left, so this is fine in a feature patch. If you want the
check backported, move it into 2/5 instead.