[PATCH v2 0/2] ethdev: fix out-of-bounds writes in rte_flow_conv()

[PATCH v2 0/2] ethdev: fix out-of-bounds writes in rte_flow_conv()

[James Raphael Tiovalen]

rte_flow_conv() is documented to truncate output to the caller-supplied
buffer size, but two paths handling variable-length trailing data
ignored that contract and copied the full payload whenever the
destination pointer was non-NULL. A caller passing a buffer just large
enough for the fixed-size header had adjacent memory clobbered:

- GENEVE_OPT: up to option_len * 4 bytes
- FLEX: up to 4 GiB, since src->length is a uint32_t and the API places
  no bounds on it

Patch 1 aligns the GENEVE_OPT guard with the sibling RAW branch, which
already gates its copy on the remaining buffer size.

Patch 2 plumbs the remaining buffer size into the flex-item desc_fn
callback (which previously took no size argument at all) and gates the
inner rte_memcpy() on it.

v2 fixes the merge conflict between patch 1 and the main branch.

James Raphael Tiovalen (2):
  ethdev: fix out-of-bounds write in GENEVE option conversion
  ethdev: fix out-of-bounds write in flex item conversion

 lib/ethdev/rte_flow.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] ethdev: fix out-of-bounds write in GENEVE option conversion

[James Raphael Tiovalen]

rte_flow_conv_item_spec() is documented to truncate output to the
caller-supplied buffer size. For RTE_FLOW_ITEM_TYPE_GENEVE_OPT, the
deep-copy of the variable-length option data was gated on `size > 0`
instead of `size >= off + tmp`, the form used by the sibling RAW
branch. A caller passing a buffer just large enough for the header
struct had adjacent memory clobbered by up to `option_len * 4` bytes of
option payload.

Align the GENEVE_OPT guard with the RAW one.

Fixes: 841a0445442d ("ethdev: fix GENEVE option item conversion")
Cc: stable@dpdk.org

Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
---
 lib/ethdev/rte_flow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index ec0fe08355..e534f2295b 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -701,7 +701,7 @@ rte_flow_conv_item_spec(void *buf, const size_t size,
 		src.geneve_opt = data;
 		dst.geneve_opt = buf;
 		tmp = spec.geneve_opt ? (spec.geneve_opt->option_len << 2) : 0;
-		if (size > 0 && tmp > 0 && src.geneve_opt->data) {
+		if (size >= off + tmp && tmp > 0 && src.geneve_opt->data) {
 			deep_src = (void *)((uintptr_t)(dst.geneve_opt + 1));
 			dst.geneve_opt->data = rte_memcpy(deep_src,
 							  src.geneve_opt->data,
-- 
2.43.0

[PATCH v2 2/2] ethdev: fix out-of-bounds write in flex item conversion

[James Raphael Tiovalen]

rte_flow_item_flex_conv() is dispatched from rte_flow_conv_copy() to
deep-copy the variable-length pattern that follows a flex item header.
The function took no size argument at all, so the trailing rte_memcpy()
of `src->length` bytes was gated only on `buf != NULL`, violating the
documented contract that output is truncated to the caller-supplied
buffer size. A caller passing a buffer just large enough for the header
struct had adjacent memory clobbered by up to 4 GiB of pattern data,
since `src->length` is uint32_t and unbounded.

Propagate the remaining buffer size `size - sz` from
rte_flow_conv_copy() into the desc_fn callback and gate the inner
memcpy on it.

Fixes: dc4d860e8a89 ("ethdev: introduce configurable flexible item")
Cc: stable@dpdk.org

Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
---
 lib/ethdev/rte_flow.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index e534f2295b..60c9a3d06f 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -36,7 +36,7 @@ uint64_t rte_flow_dynf_metadata_mask;
 struct rte_flow_desc_data {
 	const char *name;
 	size_t size;
-	size_t (*desc_fn)(void *dst, const void *src);
+	size_t (*desc_fn)(void *dst, const void *src, size_t size);
 };
 
 /**
@@ -68,16 +68,17 @@ rte_flow_conv_copy(void *buf, const void *data, const size_t size,
 	if (buf != NULL)
 		rte_memcpy(buf, data, (size > sz ? sz : size));
 	if (rte_type && desc[type].desc_fn)
-		sz += desc[type].desc_fn(size > 0 ? buf : NULL, data);
+		sz += desc[type].desc_fn(size > 0 ? buf : NULL, data,
+					 size > sz ? size - sz : 0);
 	return sz;
 }
 
 static size_t
-rte_flow_item_flex_conv(void *buf, const void *data)
+rte_flow_item_flex_conv(void *buf, const void *data, size_t size)
 {
 	struct rte_flow_item_flex *dst = buf;
 	const struct rte_flow_item_flex *src = data;
-	if (buf) {
+	if (buf && size >= src->length) {
 		dst->pattern = rte_memcpy
 			((void *)((uintptr_t)(dst + 1)), src->pattern,
 			 src->length);
-- 
2.43.0