This is a special device that's created dynamically and is supposed to stay in memory forever. We also currently don't have a devlink between it and the actual reset consumer. Suppress sysfs bind attributes so that user-space can't unbind the device because - as of now - it will cause a use-after-free splat from any user that puts the reset control handle.
Fixes: cee544a40e44 ("reset: gpio: Add GPIO-based reset controller") Cc: stable@vger.kernel.org Signed-off-by: Bartosz Golaszewski bartosz.golaszewski@oss.qualcomm.com --- drivers/reset/reset-gpio.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/reset/reset-gpio.c b/drivers/reset/reset-gpio.c index e5512b3b596b..626c4c639c15 100644 --- a/drivers/reset/reset-gpio.c +++ b/drivers/reset/reset-gpio.c @@ -111,6 +111,7 @@ static struct auxiliary_driver reset_gpio_driver = { .id_table = reset_gpio_ids, .driver = { .name = "reset-gpio", + .suppress_bind_attrs = true, }, }; module_auxiliary_driver(reset_gpio_driver);
On 04/12/2025 10:44, Bartosz Golaszewski wrote:
This is a special device that's created dynamically and is supposed to stay in memory forever. We also currently don't have a devlink between
Not forever. If every consumer is unloaded, this can be unloaded too, no?
it and the actual reset consumer. Suppress sysfs bind attributes so that
With that reasoning every reset consumer should have suppress binds. Devlink should be created by reset controller framework so it is not this driver's fault.
git grep suppress_bind_attrs -- drivers/reset/
gives only few of such controllers.
user-space can't unbind the device because - as of now - it will cause a use-after-free splat from any user that puts the reset control handle.
Fixes: cee544a40e44 ("reset: gpio: Add GPIO-based reset controller")
Nothing to be fixed here, unless you claim that every reset provider is broken as well? What is exactly different in handling devlinks between this driver and every other reset provider?
Cc: stable@vger.kernel.org Signed-off-by: Bartosz Golaszewski bartosz.golaszewski@oss.qualcomm.com
Best regards, Krzysztof
On Thu, Dec 4, 2025 at 12:14 PM Krzysztof Kozlowski krzk@kernel.org wrote:
On 04/12/2025 10:44, Bartosz Golaszewski wrote:
This is a special device that's created dynamically and is supposed to stay in memory forever. We also currently don't have a devlink between
Not forever. If every consumer is unloaded, this can be unloaded too, no?
it and the actual reset consumer. Suppress sysfs bind attributes so that
With that reasoning every reset consumer should have suppress binds. Devlink should be created by reset controller framework so it is not this driver's fault.
Here's my reasoning: I will add a devlink but Phillipp requested some changes so I still need to resend it. It will be a bigger change than this one-liner. The reset-gpio device was also converted to auxiliary bus for v6.19 and I will also convert reset core to using fwnodes for v6.20 so we'll significantly diverge in stable branches, while this issue is present ever since the reset-gpio driver exists. It's not the driver's fault but it's easier to fix it here and it very much is a special case - it's a software based device rammed in between two firmware-described devices.
user-space can't unbind the device because - as of now - it will cause a use-after-free splat from any user that puts the reset control handle.
Fixes: cee544a40e44 ("reset: gpio: Add GPIO-based reset controller")
Nothing to be fixed here, unless you claim that every reset provider is broken as well? What is exactly different in handling devlinks between this driver and every other reset provider?
I don't care if we keep the tag, it's just that this commit introduced a way for user-space to crash the system by simply unbinding reset-gpio and then its active consumers.
And the difference here is that there is no devlink between reset-gpio and its consumers. We need to first agree how to add it.
Bartosz
On 04/12/2025 12:22, Bartosz Golaszewski wrote:
On Thu, Dec 4, 2025 at 12:14 PM Krzysztof Kozlowski krzk@kernel.org wrote:
On 04/12/2025 10:44, Bartosz Golaszewski wrote:
This is a special device that's created dynamically and is supposed to stay in memory forever. We also currently don't have a devlink between
Not forever. If every consumer is unloaded, this can be unloaded too, no?
it and the actual reset consumer. Suppress sysfs bind attributes so that
With that reasoning every reset consumer should have suppress binds. Devlink should be created by reset controller framework so it is not this driver's fault.
Here's my reasoning: I will add a devlink but Phillipp requested some changes so I still need to resend it. It will be a bigger change than this one-liner. The reset-gpio device was also converted to auxiliary bus for v6.19 and I will also convert reset core to using fwnodes for v6.20 so we'll significantly diverge in stable branches, while this issue is present ever since the reset-gpio driver exists. It's not the driver's fault but it's easier to fix it here and it very much is a special case - it's a software based device rammed in between two firmware-described devices.
That's not the answer to my question. You can unbind every other reset controller. Why is this special although maybe you mentioned below?
user-space can't unbind the device because - as of now - it will cause a use-after-free splat from any user that puts the reset control handle.
Fixes: cee544a40e44 ("reset: gpio: Add GPIO-based reset controller")
Nothing to be fixed here, unless you claim that every reset provider is broken as well? What is exactly different in handling devlinks between this driver and every other reset provider?
I don't care if we keep the tag, it's just that this commit introduced a way for user-space to crash the system by simply unbinding reset-gpio and then its active consumers.
And the difference here is that there is no devlink between reset-gpio and its consumers. We need to first agree how to add it.
So you mean that between every other reset consumer and reset provider there is a devlink? And here there is no devlink?
Best regards, Krzysztof
On Thu, Dec 4, 2025 at 12:53 PM Krzysztof Kozlowski krzk@kernel.org wrote:
On 04/12/2025 12:22, Bartosz Golaszewski wrote:
On Thu, Dec 4, 2025 at 12:14 PM Krzysztof Kozlowski krzk@kernel.org wrote:
On 04/12/2025 10:44, Bartosz Golaszewski wrote:
This is a special device that's created dynamically and is supposed to stay in memory forever. We also currently don't have a devlink between
Not forever. If every consumer is unloaded, this can be unloaded too, no?
it and the actual reset consumer. Suppress sysfs bind attributes so that
With that reasoning every reset consumer should have suppress binds. Devlink should be created by reset controller framework so it is not this driver's fault.
Here's my reasoning: I will add a devlink but Phillipp requested some changes so I still need to resend it. It will be a bigger change than this one-liner. The reset-gpio device was also converted to auxiliary bus for v6.19 and I will also convert reset core to using fwnodes for v6.20 so we'll significantly diverge in stable branches, while this issue is present ever since the reset-gpio driver exists. It's not the driver's fault but it's easier to fix it here and it very much is a special case - it's a software based device rammed in between two firmware-described devices.
That's not the answer to my question. You can unbind every other reset controller. Why is this special although maybe you mentioned below?
Well, for one: when you unbind the device, it's never removed from the reset-gpio lookup list, the next consumer will never be able to get its reset control again. I recall seeing either a comment, an email or a commit message saying that this device stays in memory forever so my point stands: there's no reason to allow unbinding it. The kernel will never do it, nor should user-space.
I don't care if we keep the tag, it's just that this commit introduced a way for user-space to crash the system by simply unbinding reset-gpio and then its active consumers.
And the difference here is that there is no devlink between reset-gpio and its consumers. We need to first agree how to add it.
So you mean that between every other reset consumer and reset provider there is a devlink? And here there is no devlink?
Effectively: yes, but only because reset is OF-only (as of commit 8bffbfdc01df ("reset: remove legacy reset lookup code")) so device links are created from device-tree. If you look at any device using reset-gpio in sysfs, you'll see a supplier:gpiochipX entry but no entry for the reset. So the answer is: yes, there's no devlink between the reset-gpio provider and its consumers unless we create them explicitly, which we should eventually do but I need to rethink the patch because we should possibly also remove the devlink between the gpiochip and the reset consumer as well.
Of course, here we could mention a different story: reset seems like one of these subsystems suffering from object lifetime issues: if you remove the supplier and the consumer tries to use the reset, you'll crash because of the dangling rstc->rcdev pointer. That should be addressed as well eventually.
Bartosz
On 06/12/2025 09:45, Bartosz Golaszewski wrote:
And the difference here is that there is no devlink between reset-gpio and its consumers. We need to first agree how to add it.
So you mean that between every other reset consumer and reset provider there is a devlink? And here there is no devlink?
Effectively: yes, but only because reset is OF-only (as of commit 8bffbfdc01df ("reset: remove legacy reset lookup code")) so device links are created from device-tree. If you look at any device using reset-gpio in sysfs, you'll see a supplier:gpiochipX entry but no entry for the reset. So the answer is: yes, there's no devlink between the reset-gpio provider and its consumers unless we create them explicitly, which we should eventually do but I need to rethink the patch because we should possibly also remove the devlink between the gpiochip and the reset consumer as well.
Of course, here we could mention a different story: reset seems like one of these subsystems suffering from object lifetime issues: if you remove the supplier and the consumer tries to use the reset, you'll crash because of the dangling rstc->rcdev pointer. That should be addressed as well eventually.
Yeah, I get it. We talked in person, so unless you plan to rework the patch to fix it differently:
Reviewed-by: Krzysztof Kozlowski krzysztof.kozlowski@oss.qualcomm.com
Best regards, Krzysztof
linux-stable-mirror@lists.linaro.org
-
Bartosz Golaszewski -
Bartosz Golaszewski -
Krzysztof Kozlowski