Re: [PATCH v12 01/22] media: mc: Add INTERNAL pad flag

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The fallback logic in `media_entity_get_fwnode_pad()` was not updated to ignore internal pads, mapping external endpoints to internal pads.

Pre-existing issues:
- [High] The error path in `media_create_pad_link()` causes `num_links` counters for both the source and sink pads to underflow.
--

commit fcec98f088400370f507804a947df270add985d6
Author: Sakari Ailus <sakari.ailus@linux.intel.com>

media: mc: Add INTERNAL pad flag

Internal sink pads will be used as routing endpoints in V4L2 [GS]_ROUTING
IOCTLs, to indicate that the stream begins in the entity. Internal sink
pads are pads that have both SINK and INTERNAL flags set.

Also prevent creating links to pads that have been flagged as internal and
initialising SOURCE pads with INTERNAL flag set.

> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> index 3fa0bc687851b..7938f785e2e3c 100644
> --- a/drivers/media/mc/mc-entity.c
> +++ b/drivers/media/mc/mc-entity.c
[ ... ]
> @@ -1148,6 +1154,9 @@ media_create_pad_link(struct media_entity *source, u16 source_pad,
>  		return -EINVAL;
>  	if (WARN_ON(!(sink->pads[sink_pad].flags & MEDIA_PAD_FL_SINK)))
>  		return -EINVAL;
> +	if (WARN_ON(source->pads[source_pad].flags & MEDIA_PAD_FL_INTERNAL) ||
> +	    WARN_ON(sink->pads[sink_pad].flags & MEDIA_PAD_FL_INTERNAL))
> +		return -EINVAL;
>  
>  	link = media_add_link(&source->links);
>  	if (link == NULL)

[Severity: High]
This is a pre-existing issue, but does the error path in media_create_pad_link()
cause the num_links counters for both the source and sink pads to underflow?

If the forward link is allocated but the backlink allocation fails, the error
path calls __media_entity_remove_link(source, link):

drivers/media/mc/mc-entity.c:media_create_pad_link() {
    ...
	backlink = media_add_link(&sink->links);
	if (backlink == NULL) {
		__media_entity_remove_link(source, link);
		return -ENOMEM;
	}
    ...
}

Inside __media_entity_remove_link(), it unconditionally decrements
link->source->num_links and link->sink->num_links for data links:

drivers/media/mc/mc-entity.c:__media_entity_remove_link() {
    ...
	if ((link->flags & MEDIA_LNK_FL_LINK_TYPE) == MEDIA_LNK_FL_DATA_LINK) {
		link->source->num_links--;
		link->sink->num_links--;
    ...
}

Because these struct media_pad link counters are only incremented at the very
end of media_create_pad_link(), they haven't been incremented yet. Will these
decrements cause the unsigned 16-bit integers to underflow to 65535, corrupting
the internal state of the media graph?

> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 1c80b1d6bbaf3..80cfd12a43fc1 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -208,6 +208,7 @@ struct media_entity_desc {
>  #define MEDIA_PAD_FL_SINK			(1U << 0)
>  #define MEDIA_PAD_FL_SOURCE			(1U << 1)
>  #define MEDIA_PAD_FL_MUST_CONNECT		(1U << 2)
> +#define MEDIA_PAD_FL_INTERNAL			(1U << 3)
>  
>  struct media_pad_desc {
>  	__u32 entity;		/* entity ID */

[Severity: Medium]
Was the fallback logic in media_entity_get_fwnode_pad() updated to ignore
internal pads?

Since internal sink pads also have the MEDIA_PAD_FL_SINK flag set, the fallback
logic (used when a driver lacks a get_fwnode_pad operation) searches for the
first pad matching the requested direction flags:

drivers/media/mc/mc-entity.c:media_entity_get_fwnode_pad() {
    ...
	if (!entity->ops || !entity->ops->get_fwnode_pad) {
		for (i = 0; i < entity->num_pads; i++) {
			if (entity->pads[i].flags & direction_flags)
				return i;
		}
		return -ENXIO;
	}
    ...
}

If an internal pad appears before external pads in the array, won't it
incorrectly match and return the internal pad?

Any subsequent attempt to establish a link to this fwnode pad (e.g., via
v4l2_create_fwnode_links_to_pad()) would then trigger the new WARN_ON in
media_create_pad_link() and fail with -EINVAL, breaking initialization for
new drivers utilizing internal pads.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-gmsl2-3_serdes-v12-0-b26d92931196@analog.com?part=1