Re: [PATCH v14 3/5] rpmsg: core: match rpmsg device IDs by prefix

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Forcing global prefix matching on all rpmsg drivers without a 'longest-prefix match' rule can cause unintended device binding and collisions.
- [Medium] Module auto-loading via udev is broken for dynamically suffixed devices because module alias generation remains exact and lacks a trailing wildcard.
--

commit 7cf465d01393f57a212484ddaf8a931c40ec4179
Author: Shenwei Wang <shenwei.wang@nxp.com>

rpmsg: core: match rpmsg device IDs by prefix

This commit updates the rpmsg device matching logic to compare only the
length of the id->name string. This allows id_table entries to act as
prefixes, enabling drivers to bind to dynamically suffixed devices.

> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
> index e7f7831d37f89..f95bfc9965d4c 100644
> --- a/drivers/rpmsg/rpmsg_core.c
> +++ b/drivers/rpmsg/rpmsg_core.c
> @@ -414,7 +414,9 @@ ATTRIBUTE_GROUPS(rpmsg_dev);
>  static inline int rpmsg_id_match(const struct rpmsg_device *rpdev,
>  				  const struct rpmsg_device_id *id)
>  {
> -	return strncmp(id->name, rpdev->id.name, RPMSG_NAME_SIZE) == 0;
> +	size_t len = strnlen(id->name, RPMSG_NAME_SIZE);
> +
> +	return strncmp(id->name, rpdev->id.name, len) == 0;
>  }

[Severity: High]
Will this change cause unintended driver binding for existing drivers that
expect an exact match?

Because this modifies the global match behavior for all rpmsg drivers without
an opt-in flag or longest-prefix match logic, a shorter existing prefix could
hijack a new device.

For example, if a remote processor creates a channel named rpmsg-tty-control,
would it be incorrectly matched by an existing driver registered as rpmsg-tty?

Looking at rpmsg_dev_match(), the iteration returns on the first successful
match:

drivers/rpmsg/rpmsg_core.c:rpmsg_dev_match() {
        ...
        for (i = 0; ids[i].name[0]; i++)
                if (rpmsg_id_match(rpdev, &ids[i])) {
                        rpdev->id.driver_data = ids[i].driver_data;
                        return 1;
                }
        ...
}

Could this result in a collision if multiple IDs share the same prefix?


[Severity: Medium]
Does this change break module auto-loading via udev for dynamically suffixed
devices?

When a dynamically suffixed device is registered, rpmsg_uevent() emits the
exact name:

drivers/rpmsg/rpmsg_core.c:rpmsg_uevent() {
        ...
        return add_uevent_var(env, "MODALIAS=" RPMSG_DEVICE_MODALIAS_FMT,
                                        rpdev->id.name);
}

However, the module alias generated by file2alias.c does not append a
wildcard for prefix matching:

scripts/mod/file2alias.c:do_rpmsg_entry() {
        ...
        module_alias_printf(mod, false, RPMSG_DEVICE_MODALIAS_FMT, *name);
}

Since module_alias_printf() is called with append_wildcard set to false, the
generated alias will be an exact match (e.g., rpmsg:rpmsg-io-) which won't
match the uevent string (e.g., MODALIAS=rpmsg:rpmsg-io-1). 

Will this prevent the module from loading automatically?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625155432.815185-1-shenwei.wang@oss.nxp.com?part=3