Re: [PATCH RFC v2 05/13] sysctl, parport: update register_sysctl() callers with template arguments
Mauricio Faria de Oliveira <[email protected]>
| Newsgroups | dev.linux.lists.mptcp,dev.linux.lists.sashiko-reviews,org.kernel.vger.bpf,org.kernel.vger.linux-s390 |
|---|---|
| Message-ID | <[email protected]> |
On 2026-08-18 23:44, [email protected] wrote: > 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. > -- Pre-existing and apparently acknowledged, per details below. > > --- 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? -- Mauricio