DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] net/cpfl: validate fieldvector offsets before copying keys
@ 2026-03-21  2:16 Pengpeng Hou
  2026-08-16 22:33 ` Stephen Hemminger
  0 siblings, 1 reply; 2+ messages in thread
From: Pengpeng Hou @ 2026-03-21  2:16 UTC (permalink / raw)
  To: dev; +Cc: Praveen Shetty, pengpeng

The CPFL JSON parser accepts fieldvector offsets and SEM key sizes straight from the input description. Reject offsets that would write past the 64-byte SEM fieldvector storage and reject key sizes that would later overread the fixed source buffer or overflow the destination key buffer.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/net/intel/cpfl/cpfl_flow_engine_fxp.c |  6 ++++++
 drivers/net/intel/cpfl/cpfl_flow_parser.c     | 12 ++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/intel/cpfl/cpfl_flow_engine_fxp.c b/drivers/net/intel/cpfl/cpfl_flow_engine_fxp.c
index 361827c..d0bd909 100644
--- a/drivers/net/intel/cpfl/cpfl_flow_engine_fxp.c
+++ b/drivers/net/intel/cpfl/cpfl_flow_engine_fxp.c
@@ -173,6 +173,12 @@ cpfl_fxp_parse_pattern(const struct cpfl_flow_pr_action *pr_action,
 	if (pr_action->type == CPFL_JS_PR_ACTION_TYPE_SEM) {
 		struct cpfl_rule_info *rinfo = &rim->rules[i];
 
+		if (pr_action->sem.keysize > sizeof(pr_action->sem.cpfl_flow_pr_fv) ||
+		    pr_action->sem.keysize > sizeof(rinfo->sem.key)) {
+			PMD_DRV_LOG(ERR, "Invalid SEM key size.");
+			return false;
+		}
+
 		rinfo->type = CPFL_RULE_TYPE_SEM;
 		rinfo->sem.prof_id = pr_action->sem.prof;
 		rinfo->sem.sub_prof_id = pr_action->sem.subprof;
diff --git a/drivers/net/intel/cpfl/cpfl_flow_parser.c b/drivers/net/intel/cpfl/cpfl_flow_parser.c
index e7deb61..7fb2e7e 100644
--- a/drivers/net/intel/cpfl/cpfl_flow_parser.c
+++ b/drivers/net/intel/cpfl/cpfl_flow_parser.c
@@ -323,6 +323,10 @@ cpfl_flow_js_pattern_act_fv_proto(json_t *ob_value, struct cpfl_flow_js_fv *js_f
 		PMD_DRV_LOG(ERR, "Can not parse 'offset'.");
 		return -EINVAL;
 	}
+	if (offset >= CPFL_JS_SEM_FV_KEY_NUM_MAX / 2) {
+		PMD_DRV_LOG(ERR, "The 'offset' is too large.");
+		return -EINVAL;
+	}
 	ret = cpfl_json_t_to_uint16(ob_value, "mask", &mask);
 	if (ret < 0) {
 		PMD_DRV_LOG(ERR, "Can not parse 'mask'.");
@@ -391,6 +395,10 @@ cpfl_flow_js_pattern_act_fv(json_t *ob_fvs, struct cpfl_flow_js_pr_action *js_ac
 			PMD_DRV_LOG(ERR, "Can not parse 'offset'.");
 			goto err;
 		}
+		if (offset >= CPFL_JS_SEM_FV_KEY_NUM_MAX / 2) {
+			PMD_DRV_LOG(ERR, "The 'offset' is too large.");
+			goto err;
+		}
 		js_fv->offset = offset;
 
 		type = cpfl_json_t_to_string(object, "type");
@@ -454,6 +462,10 @@ cpfl_flow_js_pattern_per_act(json_t *ob_per_act, struct cpfl_flow_js_pr_action *
 			PMD_DRV_LOG(ERR, "Can not parse 'keysize'.");
 			return -EINVAL;
 		}
+		if (js_act->sem.keysize > sizeof(js_act->sem.cpfl_flow_pr_fv)) {
+			PMD_DRV_LOG(ERR, "The 'keysize' is too large.");
+			return -EINVAL;
+		}
 		ob_fvs = json_object_get(ob_sem, "fieldvectors");
 		ret = cpfl_flow_js_pattern_act_fv(ob_fvs, js_act);
 		if (ret < 0)
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH 2/2] net/cpfl: validate fieldvector offsets before copying keys
  2026-03-21  2:16 [PATCH 2/2] net/cpfl: validate fieldvector offsets before copying keys Pengpeng Hou
@ 2026-08-16 22:33 ` Stephen Hemminger
  0 siblings, 0 replies; 2+ messages in thread
From: Stephen Hemminger @ 2026-08-16 22:33 UTC (permalink / raw)
  To: Pengpeng Hou; +Cc: dev, Praveen Shetty

On Sat, 21 Mar 2026 10:16:34 +0800
Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:

> The CPFL JSON parser accepts fieldvector offsets and SEM key sizes straight from the input description. Reject offsets that would write past the 64-byte SEM fieldvector storage and reject key sizes that would later overread the fixed source buffer or overflow the destination key buffer.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---

This patch does not build which makes it a complete NAK.

Also, lots of AI reported issues.


Patch 2/2: net/cpfl: validate fieldvector offsets

Error: does not compile

  In cpfl_flow_js_pattern_per_act():

	if (js_act->sem.keysize > sizeof(js_act->sem.cpfl_flow_pr_fv)) {

  js_act is struct cpfl_flow_js_pr_action *, so js_act->sem is
  struct cpfl_flow_js_pr_action_sem, whose members are prof,
  subprof, keysize, fv and fv_size.  There is no cpfl_flow_pr_fv
  member.  That name belongs to struct cpfl_flow_pr_action_sem, a
  different type (cpfl_flow_parser.h:236-241).  The JSON spec
  structure and the runtime action structure have been confused.
  The intended bound is CPFL_JS_SEM_FV_KEY_NUM_MAX.

Error: the fv_proto offset check bounds the wrong value and is
       discarded

  The check added to cpfl_flow_js_pattern_act_fv_proto() applies to
  the offset stored in js_fv->proto.offset.  That is a byte offset
  into the matched rte_flow item's spec buffer, consumed in
  cpfl_parse_fv_protocol() as

	pointer = &(((const uint8_t *)(items[j].spec))[v_offset]);

  It is not a field-vector index, so bounding it by
  CPFL_JS_SEM_FV_KEY_NUM_MAX / 2 rejects legitimate protocol
  offsets while preventing no overflow.

  It also has no effect at all: both callers ignore the return
  value.

	cpfl_flow_js_pattern_act_fv_proto(ob_value, js_fv);
	cpfl_flow_js_pattern_act_fv_proto(cjson_value, js_fv);

  in cpfl_flow_js_pattern_act_fv() and
  cpfl_flow_js_pattern_act_fv_lem().  The same is true of
  cpfl_flow_js_pattern_act_fv_metadata().  Those unchecked returns
  are a pre-existing bug worth fixing, but they mean this hunk is
  dead code as written.

  The check added in cpfl_flow_js_pattern_act_fv() is the correct
  one: js_fv->offset is what indexes fv[2 * offset] and
  fv[2 * offset + 1] in cpfl_parse_fieldvectors(), and the SEM
  vector is CPFL_JS_SEM_FV_KEY_NUM_MAX (64) bytes, so offset < 32
  is right.

Error: the LEM path has the identical bugs and is left unfixed

  cpfl_flow_js_pattern_act_fv_lem() reads js_fv->offset with no
  bound.  cpfl_parse_fieldvectors() writes fv[2 * offset] and
  fv[2 * offset + 1] into pr_action->lem.cpfl_flow_pr_fv, which is
  CPFL_JS_LEM_FV_KEY_NUM_MAX (32) bytes, so the LEM bound is
  offset < 16, not 32.

  cpfl_fxp_parse_pattern() likewise guards only the SEM branch:

	memcpy(rinfo->lem.key, pr_action->lem.cpfl_flow_pr_fv,
	       rinfo->lem.key_byte_len);

  key_byte_len comes from the unvalidated uint16_t lem.keysize, the
  source is 32 bytes and rinfo->lem.key is 128, so this both
  overreads the source by up to ~64 KB and overflows the
  destination.  That is the same bug the patch fixes for SEM.

Info: second condition in the SEM check is unreachable

	if (pr_action->sem.keysize > sizeof(pr_action->sem.cpfl_flow_pr_fv) ||
	    pr_action->sem.keysize > sizeof(rinfo->sem.key)) {

  cpfl_flow_pr_fv is 64 bytes, rinfo->sem.key is
  MEV_SEM_RULE_KEY_SIZE (128).  The first condition always fires
  first; the second can be dropped.

Warning: missing Fixes: and Cc: stable@dpdk.org

  Fixes: 41f20298ee8c ("net/cpfl: parse flow offloading hint from JSON")

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-16 22:33 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-21  2:16 [PATCH 2/2] net/cpfl: validate fieldvector offsets before copying keys Pengpeng Hou
2026-08-16 22:33 ` Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox