Re: [PATCH] usb-serial: fix lifetime and locking problems

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



(Please direct tty layer patches to me as well as Greg as I'm the one
currently trying to beat the tty layer into submission)

> Similar patches will be submitted for the .stable kernels after this 
> one is accepted.  Because of other changes, this one can't be applied 
> verbatim.

This entire code area has been significantly revamped in the devel code
to use the core tty_port_* helper functions.

I'll go over this patch in more detail tomorrow however.

> -	usb_serial_put(port->serial);
> +	put_device(&port->dev);
> +
> +	/* Mustn't dereference port any more */
> +	if (count == 0) {

So why is the put_device just before this safe - it dereferences port ?

> +	 * Stop all the traffic before cancelling the work, so that
> +	 * nobody will restart it by calling usb_serial_port_softint.
> +	 */
>  	kill_traffic(port);
> +	cancel_work_sync(&port->work);

If the work queue held a kref and the destruction was done off the kref
destructor that wouldn't be a problem and indeed almost all the existing
mess goes away if we treat the tty port kref as the anchor for all the
upper levels (and the USB serial device level destructor can free a kref
to the tty to hold it all together).

The easiest way to handle this is to kill the entire disc_mutex. It
serves no real purpose except to serialize a one shot event and report
it. You can do that with test_and_set_bit()

In usb_serial_disconnect()

	if(test_and_set_bit(...))
		return;

and test the bit in the disconnect path

The other case it occurs in the old drivers is close() but this is a
symptom of incomplete implementation and non-POSIX behaviour. Once you
use the tty_port helpers then hangup() completes the physical close and
the close() method for a port post hangup is a physical layer no-op. Thus
the two flows you have are

           usb_serial_disconnect
                  t&sb blocks recursion
                        calls hangup
                             kills hardware in the recursion blocked path

            close
                tests bit for usbautopm

with usbautopm doing the disconnected device logic internally.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux