Re: [PATCH 2/2] net/cpfl: validate fieldvector offsets before copying keys

On Sat, 21 Mar 2026 10:16:34 +0800
Pengpeng Hou <[email protected]> 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 <[email protected]>
> ---

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: [email protected]

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