parport: Fix race between port and client registration

The parport subsystem registers port devices before they are fully
initialised, resulting in a race condition where client drivers such
as lp can attach to ports that are not completely initialised or even
being torn down.

When the port and client drivers are built as modules and loaded
around the same time during boot, this occasionally results in a
crash.  I was able to make this happen reliably in a VM with a
PC-style parallel port by patching parport_pc to fail probing:

> --- a/drivers/parport/parport_pc.c
> +++ b/drivers/parport/parport_pc.c
> @@ -2069,7 +2069,7 @@ static struct parport *__parport_pc_probe_port(unsigned long int base,
>  	if (!p)
>  		goto out3;
>
> -	base_res = request_region(base, 3, p->name);
> +	base_res = NULL;
>  	if (!base_res)
>  		goto out4;
>

and then running:

    while true; do
        modprobe lp & modprobe parport_pc
	wait
	rmmod lp parport_pc
    done

for a few seconds.

In the long term I think port registration should be changed to put
the call to device_add() inside parport_announce_port(), but since the
latter currently cannot fail this will require changing all port
drivers.

For now, add a flag to indicate whether a port has been "announced"
and only try to attach client drivers to ports when the flag is set.

Fixes: 6fa45a226897 ("parport: add device-model to parport subsystem")
Closes: https://bugs.debian.org/1130365
Closes: https://lore.kernel.org/all/6ba903ad-9897-42bb-8c2d-337385cc3746@molgen.mpg.de/
Cc: stable <stable@kernel.org>
Signed-off-by: Ben Hutchings <benh@debian.org>
Acked-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Link: https://patch.msgid.link/afo6uBv68GDevbMD@decadent.org.uk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/parport/share.c b/drivers/parport/share.c
index ba5292828703f202ac32d906cc28543ce8651e10..eb0977ca16053cb89b4517e01cf90d1117758a6c 100644 (file)
--- a/drivers/parport/share.c
+++ b/drivers/parport/share.c
@@ -214,10 +214,14 @@ static void get_lowlevel_driver(void)
 static int port_check(struct device *dev, void *dev_drv)
 {
        struct parport_driver *drv = dev_drv;
+       struct parport *port;
 
        /* only send ports, do not send other devices connected to bus */
-       if (is_parport(dev))
-               drv->match_port(to_parport_dev(dev));
+       if (is_parport(dev)) {
+               port = to_parport_dev(dev);
+               if (test_bit(PARPORT_ANNOUNCED, &port->devflags))
+                       drv->match_port(port);
+       }
        return 0;
 }
 
@@ -532,6 +536,7 @@ void parport_announce_port(struct parport *port)
                if (slave)
                        attach_driver_chain(slave);
        }
+       set_bit(PARPORT_ANNOUNCED, &port->devflags);
        mutex_unlock(&registration_lock);
 }
 EXPORT_SYMBOL(parport_announce_port);
@@ -561,6 +566,8 @@ void parport_remove_port(struct parport *port)
 
        mutex_lock(&registration_lock);
 
+       clear_bit(PARPORT_ANNOUNCED, &port->devflags);
+
        /* Spread the word. */
        detach_driver_chain(port);
 

diff --git a/include/linux/parport.h b/include/linux/parport.h
index 464c2ad280396c2a4e29cc68013568af6f1fae3f..f64cb0676e3b3426812ae4cab33c901796734dde 100644 (file)
--- a/include/linux/parport.h
+++ b/include/linux/parport.h
@@ -240,6 +240,7 @@ struct parport {
 
        unsigned long devflags;
 #define PARPORT_DEVPROC_REGISTERED     0
+#define PARPORT_ANNOUNCED              1
        struct pardevice *proc_device;  /* Currently register proc device */
 
        struct list_head full_list;