Re: [PATCH RFC v2 05/13] sysctl, parport: update register_sysctl() callers with template arguments

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

Pre-existing issues:
- [High] Lockless traversal of the `port->devices` list can lead to a use-after-free if a device is concurrently unregistered.
--

--- Patch [5]: [PATCH RFC v2 05/13] sysctl, parport: update register_sysctl() callers with template arguments ---
commit 511c6de215c54187a5a0e65c0208aa20775855cf
Author: Mauricio Faria de Oliveira <[email protected]>

sysctl, parport: update register_sysctl() callers with template arguments
    
Update callers with dynamic table/path to use template arguments.

> diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c
> index cda3221e386ed..7c048e9a973aa 100644
> --- a/drivers/parport/procfs.c
> +++ b/drivers/parport/procfs.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but could reading the active
device sysctl lead to a use-after-free if a device is concurrently
unregistered?

In drivers/parport/procfs.c:do_active_device(), the port->devices list is
traversed locklessly:

    for (dev = port->devices; dev ; dev = dev->next) {

Concurrently, parport_unregister_device() in drivers/parport/share.c can
remove a device from this list and free it:

    device_unregister(&dev->dev);

Since there is no synchronization (like taking port->pardevice_lock)
between the sysctl read and device removal, couldn't the lockless traversal
dereference a freed pardevice pointer?

This missing locking is also explicitly annotated in another lockless
traversal in drivers/parport/share.c:parport_release():

    /* !!! LOCKING IS NEEDED HERE */
    for (pd = port->devices; !port->cad && pd; pd = pd->next) {

Could an unprivileged user trigger this by repeatedly reading the sysctl
while a device is unregistered?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5