On Thu, Mar 28, 2019 at 01:02:50PM +0100, Greg KH wrote:
> On Thu, Mar 28, 2019 at 02:52:08PM +0300, Heikki Krogerus wrote:
> > It is possible that the driver of the mux device has been
> > unbind by the time typec_mux_put() or typec_switch_put() is
> > called.
> >
> > To prevent the NULL Pointer Dereference from happening in
> > this case when decrementing the reference count of the
> > module by using dev->driver->owner, storing the module
> > handle to the mux and switch data structures, and using the
> > stored value instead.
> >
> > Fixes: ("3e3b81965cbf usb: typec: mux: Take care of driver module reference counting")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> > ---
> > drivers/usb/typec/mux.c | 10 ++++++----
> > include/linux/usb/typec_mux.h | 2 ++
> > 2 files changed, 8 insertions(+), 4 deletions(-)
> >
> > --- a/include/linux/usb/typec_mux.h
> > +++ b/include/linux/usb/typec_mux.h
> > @@ -20,6 +20,7 @@ struct device;
> > */
> > struct typec_switch {
> > struct device *dev;
> > + struct module *module;
> > struct list_head entry;
> >
> > int (*set)(struct typec_switch *sw, enum typec_orientation orientation);
> > @@ -37,6 +38,7 @@ struct typec_switch {
> > */
> > struct typec_mux {
> > struct device *dev;
> > + struct module *module;
> > struct list_head entry;
>
> You have just created two different reference counts for a single object
> here :(
>
> This is data, not code. Data needs to be protected with the reference
> count to keep it from being freed while in use. Code also needs to be
> protected, BUT, do not mix the two.
>
> driver owners should always be properly reference counted if userspace
> opens the device, not if another internal kernel code opens the device.
> Or better yet, just properly unwind things when the code is removed, no
> need to reference count anything on the module level (like networking
> devices do it).
>
> So I really do not think this patch is correct, and odds are, the
> original one that this patch says it fixes is probably also not correct
> :(
OK. I'll see if I can prepare a proper fix for the original.
thanks,
--
heikki
