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=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 6F0B2C43381 for ; Thu, 28 Mar 2019 14:56:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 475382173C for ; Thu, 28 Mar 2019 14:56:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726076AbfC1O46 (ORCPT ); Thu, 28 Mar 2019 10:56:58 -0400 Received: from mga06.intel.com ([134.134.136.31]:46905 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725816AbfC1O46 (ORCPT ); Thu, 28 Mar 2019 10:56:58 -0400 X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Mar 2019 07:56:57 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,280,1549958400"; d="scan'208";a="159246168" Received: from kuha.fi.intel.com ([10.237.72.189]) by fmsmga001.fm.intel.com with SMTP; 28 Mar 2019 07:56:55 -0700 Received: by kuha.fi.intel.com (sSMTP sendmail emulation); Thu, 28 Mar 2019 16:56:54 +0200 Date: Thu, 28 Mar 2019 16:56:54 +0200 From: Heikki Krogerus To: Greg KH Cc: Hans de Goede , linux-usb@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH] usb: typec: mux: Store module handle Message-ID: <20190328145654.GB9993@kuha.fi.intel.com> References: <20190328115208.42086-1-heikki.krogerus@linux.intel.com> <20190328120250.GA6819@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190328120250.GA6819@kroah.com> User-Agent: Mutt/1.11.3 (2019-02-01) Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org 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@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 > :( OK. I'll see if I can prepare a proper fix for the original. thanks, -- heikki 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: Heikki Krogerus Message-Id: <20190328145654.GB9993@kuha.fi.intel.com> Date: Thu, 28 Mar 2019 16:56:54 +0200 To: Greg KH Cc: Hans de Goede , linux-usb@vger.kernel.org, stable@vger.kernel.org List-ID: T24gVGh1LCBNYXIgMjgsIDIwMTkgYXQgMDE6MDI6NTBQTSArMDEwMCwgR3JlZyBLSCB3cm90ZToK PiBPbiBUaHUsIE1hciAyOCwgMjAxOSBhdCAwMjo1MjowOFBNICswMzAwLCBIZWlra2kgS3JvZ2Vy dXMgd3JvdGU6Cj4gPiBJdCBpcyBwb3NzaWJsZSB0aGF0IHRoZSBkcml2ZXIgb2YgdGhlIG11eCBk ZXZpY2UgaGFzIGJlZW4KPiA+IHVuYmluZCBieSB0aGUgdGltZSB0eXBlY19tdXhfcHV0KCkgb3Ig dHlwZWNfc3dpdGNoX3B1dCgpIGlzCj4gPiBjYWxsZWQuCj4gPiAKPiA+IFRvIHByZXZlbnQgdGhl IE5VTEwgUG9pbnRlciBEZXJlZmVyZW5jZSBmcm9tIGhhcHBlbmluZyBpbgo+ID4gdGhpcyBjYXNl IHdoZW4gZGVjcmVtZW50aW5nIHRoZSByZWZlcmVuY2UgY291bnQgb2YgdGhlCj4gPiBtb2R1bGUg YnkgdXNpbmcgZGV2LT5kcml2ZXItPm93bmVyLCBzdG9yaW5nIHRoZSBtb2R1bGUKPiA+IGhhbmRs ZSB0byB0aGUgbXV4IGFuZCBzd2l0Y2ggZGF0YSBzdHJ1Y3R1cmVzLCBhbmQgdXNpbmcgdGhlCj4g PiBzdG9yZWQgdmFsdWUgaW5zdGVhZC4KPiA+IAo+ID4gRml4ZXM6ICgiM2UzYjgxOTY1Y2JmIHVz YjogdHlwZWM6IG11eDogVGFrZSBjYXJlIG9mIGRyaXZlciBtb2R1bGUgcmVmZXJlbmNlIGNvdW50 aW5nIikKPiA+IENjOiBzdGFibGVAdmdlci5rZXJuZWwub3JnCj4gPiBTaWduZWQtb2ZmLWJ5OiBI ZWlra2kgS3JvZ2VydXMgPGhlaWtraS5rcm9nZXJ1c0BsaW51eC5pbnRlbC5jb20+Cj4gPiAtLS0K PiA+ICBkcml2ZXJzL3VzYi90eXBlYy9tdXguYyAgICAgICB8IDEwICsrKysrKy0tLS0KPiA+ICBp bmNsdWRlL2xpbnV4L3VzYi90eXBlY19tdXguaCB8ICAyICsrCj4gPiAgMiBmaWxlcyBjaGFuZ2Vk LCA4IGluc2VydGlvbnMoKyksIDQgZGVsZXRpb25zKC0pCj4gPiAKPiA+IC0tLSBhL2luY2x1ZGUv bGludXgvdXNiL3R5cGVjX211eC5oCj4gPiArKysgYi9pbmNsdWRlL2xpbnV4L3VzYi90eXBlY19t dXguaAo+ID4gQEAgLTIwLDYgKzIwLDcgQEAgc3RydWN0IGRldmljZTsKPiA+ICAgKi8KPiA+ICBz dHJ1Y3QgdHlwZWNfc3dpdGNoIHsKPiA+ICAJc3RydWN0IGRldmljZSAqZGV2Owo+ID4gKwlzdHJ1 Y3QgbW9kdWxlICptb2R1bGU7Cj4gPiAgCXN0cnVjdCBsaXN0X2hlYWQgZW50cnk7Cj4gPiAgCj4g PiAgCWludCAoKnNldCkoc3RydWN0IHR5cGVjX3N3aXRjaCAqc3csIGVudW0gdHlwZWNfb3JpZW50 YXRpb24gb3JpZW50YXRpb24pOwo+ID4gQEAgLTM3LDYgKzM4LDcgQEAgc3RydWN0IHR5cGVjX3N3 aXRjaCB7Cj4gPiAgICovCj4gPiAgc3RydWN0IHR5cGVjX211eCB7Cj4gPiAgCXN0cnVjdCBkZXZp Y2UgKmRldjsKPiA+ICsJc3RydWN0IG1vZHVsZSAqbW9kdWxlOwo+ID4gIAlzdHJ1Y3QgbGlzdF9o ZWFkIGVudHJ5Owo+IAo+IFlvdSBoYXZlIGp1c3QgY3JlYXRlZCB0d28gZGlmZmVyZW50IHJlZmVy ZW5jZSBjb3VudHMgZm9yIGEgc2luZ2xlIG9iamVjdAo+IGhlcmUgOigKPiAKPiBUaGlzIGlzIGRh dGEsIG5vdCBjb2RlLiAgRGF0YSBuZWVkcyB0byBiZSBwcm90ZWN0ZWQgd2l0aCB0aGUgcmVmZXJl bmNlCj4gY291bnQgdG8ga2VlcCBpdCBmcm9tIGJlaW5nIGZyZWVkIHdoaWxlIGluIHVzZS4gIENv ZGUgYWxzbyBuZWVkcyB0byBiZQo+IHByb3RlY3RlZCwgQlVULCBkbyBub3QgbWl4IHRoZSB0d28u Cj4gCj4gZHJpdmVyIG93bmVycyBzaG91bGQgYWx3YXlzIGJlIHByb3Blcmx5IHJlZmVyZW5jZSBj b3VudGVkIGlmIHVzZXJzcGFjZQo+IG9wZW5zIHRoZSBkZXZpY2UsIG5vdCBpZiBhbm90aGVyIGlu dGVybmFsIGtlcm5lbCBjb2RlIG9wZW5zIHRoZSBkZXZpY2UuCj4gT3IgYmV0dGVyIHlldCwganVz dCBwcm9wZXJseSB1bndpbmQgdGhpbmdzIHdoZW4gdGhlIGNvZGUgaXMgcmVtb3ZlZCwgbm8KPiBu ZWVkIHRvIHJlZmVyZW5jZSBjb3VudCBhbnl0aGluZyBvbiB0aGUgbW9kdWxlIGxldmVsIChsaWtl IG5ldHdvcmtpbmcKPiBkZXZpY2VzIGRvIGl0KS4KPiAKPiBTbyBJIHJlYWxseSBkbyBub3QgdGhp bmsgdGhpcyBwYXRjaCBpcyBjb3JyZWN0LCBhbmQgb2RkcyBhcmUsIHRoZQo+IG9yaWdpbmFsIG9u ZSB0aGF0IHRoaXMgcGF0Y2ggc2F5cyBpdCBmaXhlcyBpcyBwcm9iYWJseSBhbHNvIG5vdCBjb3Jy ZWN0Cj4gOigKCk9LLiBJJ2xsIHNlZSBpZiBJIGNhbiBwcmVwYXJlIGEgcHJvcGVyIGZpeCBmb3Ig dGhlIG9yaWdpbmFsLgoKdGhhbmtzLAo=