linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
	linux-usb@vger.kernel.org, stable@vger.kernel.org
Subject: usb: typec: mux: Store module handle
Date: Thu, 28 Mar 2019 13:02:50 +0100	[thread overview]
Message-ID: <20190328120250.GA6819@kroah.com> (raw)

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@vger.kernel.org
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  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
:(

thanks,

greg k-h

             reply	other threads:[~2019-03-28 12:02 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-28 12:02 Greg Kroah-Hartman [this message]
  -- strict thread matches above, loose matches on Subject: below --
2019-03-29 15:05 usb: typec: mux: Store module handle Heikki Krogerus
2019-03-28 14:56 Heikki Krogerus
2019-03-28 11:52 Heikki Krogerus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190328120250.GA6819@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=hdegoede@redhat.com \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=linux-usb@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).