* [PATCH] drm/rockchip: use the safe iterator when removing device links
@ 2026-11-04 2:53 ` Wentao Liang
0 siblings, 0 replies; 5+ messages in thread
From: Wentao Liang @ 2026-11-04 2:53 UTC (permalink / raw)
To: Sandy Huang, Heiko Stübner, Andy Yan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-rockchip, linux-arm-kernel
rockchip_drm_match_remove() walks dev->links.consumers and calls
device_link_del() for every entry. That drops the link's reference and,
once it reaches zero, __device_link_del() removes the link from the
device link lists and schedules the release of the link object.
list_for_each_entry() still reads link->s_node after the loop body to
find the next entry, so it accesses a link that has already been
released. Use list_for_each_entry_safe() so that the next entry is
loaded before the current link is deleted.
Fixes: 7d4e981d41e3 ("drm/rockchip: Add device links for master and components")
Cc: stable@vger.kernel.org
Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 3099408e9d05..9edfe4db9393 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -356,9 +356,9 @@ int rockchip_drm_endpoint_is_subdriver(struct device_node *ep)
static void rockchip_drm_match_remove(struct device *dev)
{
- struct device_link *link;
+ struct device_link *link, *tmp;
- list_for_each_entry(link, &dev->links.consumers, s_node)
+ list_for_each_entry_safe(link, tmp, &dev->links.consumers, s_node)
device_link_del(link);
}
--
2.34.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] drm/rockchip: use the safe iterator when removing device links
2026-11-04 2:53 ` Wentao Liang
@ 2026-11-04 7:10 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 5+ messages in thread
From: Krzysztof Kozlowski @ 2026-11-04 7:10 UTC (permalink / raw)
To: Wentao Liang
Cc: Maxime Ripard, Andy Yan, David Airlie, dri-devel, linux-rockchip,
Heiko Stübner, Thomas Zimmermann, Sandy Huang, Simona Vetter,
Maarten Lankhorst, linux-arm-kernel
On Wed, 04 Nov 2026 02:53:46 +0000, Wentao Liang wrote:
> rockchip_drm_match_remove() walks dev->links.consumers and calls
> device_link_del() for every entry. That drops the link's reference and,
> once it reaches zero, __device_link_del() removes the link from the
> device link lists and schedules the release of the link object.
>
> list_for_each_entry() still reads link->s_node after the loop body to
> find the next entry, so it accesses a link that has already been
> released. Use list_for_each_entry_safe() so that the next entry is
> loaded before the current link is deleted.
>
> Fixes: 7d4e981d41e3 ("drm/rockchip: Add device links for master and components")
> Cc: stable@vger.kernel.org
> Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Multiple things here:
1. Your team ignored completely previous feedback. Multiple times.
2. You use multiple identities with this email address, thus I doubt that we
speak with an actual person.
3. Your bot does not even bother running checkpatch, right?
4. Nothing of this was reproduced nor tested.
5. Finally, the same feedback:
You sent multiple independent patches, to multiple independent
subsystems. The amount of these patches clearly suggest this was
AI generated and most likely not tested.
More importantly, you sent all this work without properly organizing
relevant patches into patchsets. This makes reviewing difficult
and might cause multiple reviewers to address the same issue.
Replying to the entire set is impossible and requires handling each
patch independently, instead of applying or discarding the set.
Maintainers also won't see the bigger picture of your work. Quite
worrying.
This is on the verge of hostile patch: bomb us with so many
contributions, we won't be able to handle them in efficient manner,
like responding ONCE to ask you to slow down. Considering all this
is untested and LLM generated, I have even more doubts whether this
should be considered for review.
Please read kernel documentation BEFORE posting more work. It will
explain you how to identify subsystems, how to organize your work per
subsystem (so a patchset grouping multiple patches with a short cover
letter), how to document the issue discovery method and what was
or was not tested.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm/rockchip: use the safe iterator when removing device links
@ 2026-11-04 7:10 ` Krzysztof Kozlowski
0 siblings, 0 replies; 5+ messages in thread
From: Krzysztof Kozlowski @ 2026-11-04 7:10 UTC (permalink / raw)
To: Wentao Liang
Cc: Maxime Ripard, Andy Yan, David Airlie, dri-devel, linux-rockchip,
Heiko Stübner, Thomas Zimmermann, Sandy Huang, Simona Vetter,
Maarten Lankhorst, linux-arm-kernel
On Wed, 04 Nov 2026 02:53:46 +0000, Wentao Liang wrote:
> rockchip_drm_match_remove() walks dev->links.consumers and calls
> device_link_del() for every entry. That drops the link's reference and,
> once it reaches zero, __device_link_del() removes the link from the
> device link lists and schedules the release of the link object.
>
> list_for_each_entry() still reads link->s_node after the loop body to
> find the next entry, so it accesses a link that has already been
> released. Use list_for_each_entry_safe() so that the next entry is
> loaded before the current link is deleted.
>
> Fixes: 7d4e981d41e3 ("drm/rockchip: Add device links for master and components")
> Cc: stable@vger.kernel.org
> Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Multiple things here:
1. Your team ignored completely previous feedback. Multiple times.
2. You use multiple identities with this email address, thus I doubt that we
speak with an actual person.
3. Your bot does not even bother running checkpatch, right?
4. Nothing of this was reproduced nor tested.
5. Finally, the same feedback:
You sent multiple independent patches, to multiple independent
subsystems. The amount of these patches clearly suggest this was
AI generated and most likely not tested.
More importantly, you sent all this work without properly organizing
relevant patches into patchsets. This makes reviewing difficult
and might cause multiple reviewers to address the same issue.
Replying to the entire set is impossible and requires handling each
patch independently, instead of applying or discarding the set.
Maintainers also won't see the bigger picture of your work. Quite
worrying.
This is on the verge of hostile patch: bomb us with so many
contributions, we won't be able to handle them in efficient manner,
like responding ONCE to ask you to slow down. Considering all this
is untested and LLM generated, I have even more doubts whether this
should be considered for review.
Please read kernel documentation BEFORE posting more work. It will
explain you how to identify subsystems, how to organize your work per
subsystem (so a patchset grouping multiple patches with a short cover
letter), how to document the issue discovery method and what was
or was not tested.
Best regards,
Krzysztof
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/rockchip: use the safe iterator when removing device links
2026-11-04 2:53 ` Wentao Liang
(?)
(?)
@ 2026-11-05 15:54 ` sashiko-bot
-1 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-11-05 15:54 UTC (permalink / raw)
To: Wentao Liang; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/rockchip: wrong unlocked list in rockchip_drm_match_remove()
--
commit e68a6141bb89fc9c57189db5bdc14a46b7da846f
Author: Wentao Liang <vulab@iscas.ac.cn>
drm/rockchip: use the safe iterator when removing device links
This commit changes rockchip_drm_match_remove() to use
list_for_each_entry_safe() when walking dev->links.consumers and calling
device_link_del().
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index 3099408e9d050..9edfe4db93934 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -356,9 +356,9 @@ int rockchip_drm_endpoint_is_subdriver(struct device_node *ep)
>
> static void rockchip_drm_match_remove(struct device *dev)
> {
> - struct device_link *link;
> + struct device_link *link, *tmp;
>
> - list_for_each_entry(link, &dev->links.consumers, s_node)
> + list_for_each_entry_safe(link, tmp, &dev->links.consumers, s_node)
> device_link_del(link);
[Severity: High]
Does walking dev->links.consumers in rockchip_drm_match_remove() actually
remove the stateless device links created by rockchip_drm_match_add(), or
does it walk the wrong list without holding device_links_lock?
In rockchip_drm_match_add(), the links are created with the master device dev
as the consumer and each sub-driver device d as the supplier:
device_link_add(dev, d, DL_FLAG_STATELESS);
In drivers/base/core.c:device_link_add(), s_node is added to
supplier->links.consumers (which is d->links.consumers) and c_node is added to
consumer->links.suppliers (which is dev->links.suppliers):
list_add_tail_rcu(&link->s_node, &supplier->links.consumers);
list_add_tail_rcu(&link->c_node, &consumer->links.suppliers);
Because dev is the consumer rather than the supplier, none of the links
created by rockchip_drm_match_add() are on dev->links.consumers, so they are
all leaked when rockchip_drm_platform_probe() fails or defers and when
rockchip_drm_platform_remove() runs.
In addition, dev->links.consumers and dev->links.suppliers are internal to the
driver core and protected by device_links_write_lock(). Because
device_link_del() only acquires and releases that lock inside each call,
iterating dev->links outside drivers/base/core.c leaves the list traversal and
tmp pointer unserialized against concurrent device link changes. If any
managed links (such as fw_devlink links) are present on the list, calling
device_link_del() on them will also trigger the WARN in
device_link_put_kref():
static void device_link_put_kref(struct device_link *link)
{
if (device_link_test(link, DL_FLAG_STATELESS))
kref_put(&link->kref, __device_link_del);
else if (!device_is_registered(link->consumer))
__device_link_del(&link->kref);
else
WARN(1, "Unable to drop a managed device link reference\n");
}
Should rockchip_drm_match_remove() instead iterate the sub-driver devices and
call the exported device_link_remove(dev, d) helper (or track the
struct device_link pointers returned by device_link_add())?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261104025346.1527550-1-vulab@iscas.ac.cn?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread