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.
