Re: [PATCH] usb: typec: mux: Store module handle

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

 



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



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux