From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.5 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9403DC43381 for ; Thu, 28 Mar 2019 12:02:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5408A2173C for ; Thu, 28 Mar 2019 12:02:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1553774574; bh=TtxSaBpc8DfE1BejZibsAa110jemBVZsDKsZ1VkZZOo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=2WzDDKgWm6n63BPxu/BcWPeXo+BR8XzKTZgPF2rfU51Z77qhkZarpKWZMhAtSLaBv ZXggboKwPQRu4ClwNfc4I+B4Llq4Z1M+R+lkqT9iLRXFdXnfasXZIBFnM4IqnKuk+s nIGC8E5sUdoZR1CCYHcDGY52H5XTZnISCZjL2+lA= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726243AbfC1MCx (ORCPT ); Thu, 28 Mar 2019 08:02:53 -0400 Received: from mail.kernel.org ([198.145.29.99]:60520 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726182AbfC1MCx (ORCPT ); Thu, 28 Mar 2019 08:02:53 -0400 Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 903E920700; Thu, 28 Mar 2019 12:02:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1553774573; bh=TtxSaBpc8DfE1BejZibsAa110jemBVZsDKsZ1VkZZOo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=QXJHKUbL/XY7n8xL/0bRYrq9Ve2xKLG0EwfnSY8XTqEpMmqVhPDPxynZkXNJXcwws EavJBj9iv+9CpyEf3ror4ATyY4ZOWf/2KFtRvS89OBiPz5nH5DO4vEpOmA6Vh6Ds76 HiCQs8wFBKFyubqjrjflRQDg5TTqyAen4KIscHM4= Date: Thu, 28 Mar 2019 13:02:50 +0100 From: Greg KH To: Heikki Krogerus Cc: Hans de Goede , linux-usb@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH] usb: typec: mux: Store module handle Message-ID: <20190328120250.GA6819@kroah.com> References: <20190328115208.42086-1-heikki.krogerus@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190328115208.42086-1-heikki.krogerus@linux.intel.com> User-Agent: Mutt/1.11.4 (2019-03-13) Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org 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 > --- > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: usb: typec: mux: Store module handle From: Greg Kroah-Hartman Message-Id: <20190328120250.GA6819@kroah.com> Date: Thu, 28 Mar 2019 13:02:50 +0100 To: Heikki Krogerus Cc: Hans de Goede , linux-usb@vger.kernel.org, stable@vger.kernel.org List-ID: T24gVGh1LCBNYXIgMjgsIDIwMTkgYXQgMDI6NTI6MDhQTSArMDMwMCwgSGVpa2tpIEtyb2dlcnVz IHdyb3RlOgo+IEl0IGlzIHBvc3NpYmxlIHRoYXQgdGhlIGRyaXZlciBvZiB0aGUgbXV4IGRldmlj ZSBoYXMgYmVlbgo+IHVuYmluZCBieSB0aGUgdGltZSB0eXBlY19tdXhfcHV0KCkgb3IgdHlwZWNf c3dpdGNoX3B1dCgpIGlzCj4gY2FsbGVkLgo+IAo+IFRvIHByZXZlbnQgdGhlIE5VTEwgUG9pbnRl ciBEZXJlZmVyZW5jZSBmcm9tIGhhcHBlbmluZyBpbgo+IHRoaXMgY2FzZSB3aGVuIGRlY3JlbWVu dGluZyB0aGUgcmVmZXJlbmNlIGNvdW50IG9mIHRoZQo+IG1vZHVsZSBieSB1c2luZyBkZXYtPmRy aXZlci0+b3duZXIsIHN0b3JpbmcgdGhlIG1vZHVsZQo+IGhhbmRsZSB0byB0aGUgbXV4IGFuZCBz d2l0Y2ggZGF0YSBzdHJ1Y3R1cmVzLCBhbmQgdXNpbmcgdGhlCj4gc3RvcmVkIHZhbHVlIGluc3Rl YWQuCj4gCj4gRml4ZXM6ICgiM2UzYjgxOTY1Y2JmIHVzYjogdHlwZWM6IG11eDogVGFrZSBjYXJl IG9mIGRyaXZlciBtb2R1bGUgcmVmZXJlbmNlIGNvdW50aW5nIikKPiBDYzogc3RhYmxlQHZnZXIu a2VybmVsLm9yZwo+IFNpZ25lZC1vZmYtYnk6IEhlaWtraSBLcm9nZXJ1cyA8aGVpa2tpLmtyb2dl cnVzQGxpbnV4LmludGVsLmNvbT4KPiAtLS0KPiAgZHJpdmVycy91c2IvdHlwZWMvbXV4LmMgICAg ICAgfCAxMCArKysrKystLS0tCj4gIGluY2x1ZGUvbGludXgvdXNiL3R5cGVjX211eC5oIHwgIDIg KysKPiAgMiBmaWxlcyBjaGFuZ2VkLCA4IGluc2VydGlvbnMoKyksIDQgZGVsZXRpb25zKC0pCj4g Cj4gLS0tIGEvaW5jbHVkZS9saW51eC91c2IvdHlwZWNfbXV4LmgKPiArKysgYi9pbmNsdWRlL2xp bnV4L3VzYi90eXBlY19tdXguaAo+IEBAIC0yMCw2ICsyMCw3IEBAIHN0cnVjdCBkZXZpY2U7Cj4g ICAqLwo+ICBzdHJ1Y3QgdHlwZWNfc3dpdGNoIHsKPiAgCXN0cnVjdCBkZXZpY2UgKmRldjsKPiAr CXN0cnVjdCBtb2R1bGUgKm1vZHVsZTsKPiAgCXN0cnVjdCBsaXN0X2hlYWQgZW50cnk7Cj4gIAo+ ICAJaW50ICgqc2V0KShzdHJ1Y3QgdHlwZWNfc3dpdGNoICpzdywgZW51bSB0eXBlY19vcmllbnRh dGlvbiBvcmllbnRhdGlvbik7Cj4gQEAgLTM3LDYgKzM4LDcgQEAgc3RydWN0IHR5cGVjX3N3aXRj aCB7Cj4gICAqLwo+ICBzdHJ1Y3QgdHlwZWNfbXV4IHsKPiAgCXN0cnVjdCBkZXZpY2UgKmRldjsK PiArCXN0cnVjdCBtb2R1bGUgKm1vZHVsZTsKPiAgCXN0cnVjdCBsaXN0X2hlYWQgZW50cnk7CgpZ b3UgaGF2ZSBqdXN0IGNyZWF0ZWQgdHdvIGRpZmZlcmVudCByZWZlcmVuY2UgY291bnRzIGZvciBh IHNpbmdsZSBvYmplY3QKaGVyZSA6KAoKVGhpcyBpcyBkYXRhLCBub3QgY29kZS4gIERhdGEgbmVl ZHMgdG8gYmUgcHJvdGVjdGVkIHdpdGggdGhlIHJlZmVyZW5jZQpjb3VudCB0byBrZWVwIGl0IGZy b20gYmVpbmcgZnJlZWQgd2hpbGUgaW4gdXNlLiAgQ29kZSBhbHNvIG5lZWRzIHRvIGJlCnByb3Rl Y3RlZCwgQlVULCBkbyBub3QgbWl4IHRoZSB0d28uCgpkcml2ZXIgb3duZXJzIHNob3VsZCBhbHdh eXMgYmUgcHJvcGVybHkgcmVmZXJlbmNlIGNvdW50ZWQgaWYgdXNlcnNwYWNlCm9wZW5zIHRoZSBk ZXZpY2UsIG5vdCBpZiBhbm90aGVyIGludGVybmFsIGtlcm5lbCBjb2RlIG9wZW5zIHRoZSBkZXZp Y2UuCk9yIGJldHRlciB5ZXQsIGp1c3QgcHJvcGVybHkgdW53aW5kIHRoaW5ncyB3aGVuIHRoZSBj b2RlIGlzIHJlbW92ZWQsIG5vCm5lZWQgdG8gcmVmZXJlbmNlIGNvdW50IGFueXRoaW5nIG9uIHRo ZSBtb2R1bGUgbGV2ZWwgKGxpa2UgbmV0d29ya2luZwpkZXZpY2VzIGRvIGl0KS4KClNvIEkgcmVh bGx5IGRvIG5vdCB0aGluayB0aGlzIHBhdGNoIGlzIGNvcnJlY3QsIGFuZCBvZGRzIGFyZSwgdGhl Cm9yaWdpbmFsIG9uZSB0aGF0IHRoaXMgcGF0Y2ggc2F5cyBpdCBmaXhlcyBpcyBwcm9iYWJseSBh bHNvIG5vdCBjb3JyZWN0CjooCgp0aGFua3MsCgpncmVnIGstaAo=