Re: [PATCH RFC v2 07/15] ctlv: Prepare schema validation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] ctlv: ctlv_schema_min_len() enforces record depth limit on
  leaf nodes
--

commit f7e5c872905b61fc69a0da34a919232ff36227be
Author: Jiri Pirko <jiri@resnulli.us>

ctlv: Prepare schema validation

This commit adds generated-schema walking, message validation, and request
indexing for CTLV schemas.

> diff --git a/lib/ctlv/schema.c b/lib/ctlv/schema.c
> new file mode 100644
> index 0000000000000..8ff3c7c5b5fe0
> --- /dev/null
> +++ b/lib/ctlv/schema.c
> @@ -0,0 +1,1092 @@
[ ... ]
> +/* Buffer length that the largest indivisible leaf and its records need. */
> +static u64 ctlv_schema_min_len(struct ctlv_schema_length_walk *walk,
> +			       const struct ctlv_schema_tree *tree, u32 index, u32 above, u32 depth)
> +{
> +	const struct ctlv_node_schema *node = &tree->node_schemas[index];
> +	bool has_children = false;
> +	u64 deepest = 0;
> +	u32 i;
> +
> +	if (depth > CTLV_MAX_DECLARED_RECORD_DEPTH) {
> +		walk->bad = true;
> +		return 0;
> +	}
> +
> +	for (i = 0; i < node->children_count; i++) {
> +		u32 child = node->first_child + i;
> +
> +		has_children = true;
> +		deepest = max(deepest,
> +			      ctlv_schema_min_len(walk, tree, child,
> +						  above + sizeof(struct ctlv_hdr), depth + 1));
> +	}
> +
> +	for (i = 0; i < tree->ext_schemas_count; i++) {
> +		const struct ctlv_ext_schema *ext_schema = &tree->ext_schemas[i];
> +		const struct ctlv_ext_fill_schema *fill_schema;
> +		const struct ctlv_node_schema *root;
> +		u32 member;
> +
> +		if (ext_schema->node != index)
> +			continue;
> +		fill_schema =
> +			ctlv_schema_fill(walk->family, ext_schema->family_id, ext_schema->ext_id);
> +		if (!fill_schema)
> +			continue;
> +		root = &fill_schema->members.node_schemas[0];
> +		for (member = 0; member < root->children_count; member++) {
> +			has_children = true;
> +			deepest = max(deepest, ctlv_schema_min_len(walk, &fill_schema->members,
> +								   root->first_child + member,
> +								   above + sizeof(struct ctlv_hdr),
> +								   depth + 1));
> +		}
> +	}
> +
> +	if (has_children)
> +		return deepest;
> +	/* An empty root is a message of its own header alone. */
> +	if (!index)
> +		return above + sizeof(struct ctlv_hdr);
> +	return above + ALIGN(ctlv_schema_tlv_max_len(walk, tree, index, depth), CTLV_ALIGNTO);
> +}

[Severity: Medium]
Does ctlv_schema_min_len() reject valid leaf fields inside a record at
CTLV_MAX_DECLARED_RECORD_DEPTH?

Unlike ctlv_schema_children_max_len(), which is only entered for
CTLV_SCHEMA_RECORD nodes, ctlv_schema_min_len() recurses into every child
and extension fill member with depth + 1 and checks depth >
CTLV_MAX_DECLARED_RECORD_DEPTH at entry for all nodes.  When a non-reentering
query reply has a record at depth CTLV_MAX_DECLARED_RECORD_DEPTH (7), its
leaf children are visited at depth 8, setting walk->bad = true and causing
ctlv_schema_op_sizes() to fail with -EINVAL.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113155515.2510194-1-jiri@resnulli.us?part=7