On 10/26/2026 5:51 PM, Stephen Hemminger wrote:
On Sat, 24 Oct 2026 14:50:28 +0100
Anatoly Burakov <[email protected]> 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: [email protected]

Signed-off-by: Anatoly Burakov <[email protected]>
---

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.

Thanks, these seem like real issues so I'll take a look.

--
Thanks,
Anatoly

Reply via email to