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

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 10/26/2026 5:51 PM, Stephen Hemminger wrote:
> 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.

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

-- 
Thanks,
Anatoly