Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
From:   "Zhang, Qiang1" <qiang1.zhang@intel.com>
To:     "stern@rowland.harvard.edu" <stern@rowland.harvard.edu>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
CC:     syzbot <syzbot+348b571beb5eeb70a582@syzkaller.appspotmail.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "rafael@kernel.org" <rafael@kernel.org>,
        "syzkaller-bugs@googlegroups.com" <syzkaller-bugs@googlegroups.com>,
        "balbi@kernel.org" <balbi@kernel.org>
Subject: RE: [syzbot] KASAN: use-after-free Read in dev_uevent
Thread-Topic: [syzbot] KASAN: use-after-free Read in dev_uevent
Thread-Index: AQHYJn4xBH205WNMF0CseYQWl0QGTKyg/zWggAAE7YCAADTiAIAAFuAAgAAJtACAAJZv8A==
Date:   Thu, 24 Feb 2022 01:44:11 +0000
Message-ID: <PH0PR11MB5880D7544442B4D60810F0D2DA3D9@PH0PR11MB5880.namprd11.prod.outlook.com>
References: <0000000000005a991a05a86970bb@google.com>
 <00000000000033314805d8765175@google.com>
 <PH0PR11MB58805E3C4CF7D4C41D49BFCFDA3C9@PH0PR11MB5880.namprd11.prod.outlook.com>
 <YhYafwiwUV2Sbn5t@kroah.com> <YhZG3GJb8G7oL7l7@rowland.harvard.edu>
 <YhZaDGeIIvpILdCk@kroah.com> <YhZiMHHjrBw8am5g@rowland.harvard.edu>
In-Reply-To: <YhZiMHHjrBw8am5g@rowland.harvard.edu>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-version: 11.6.200.16
dlp-reaction: no-action
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?C26oOeVdit65q4erQzyaOVvS0S8lG3MR2ShczNBkF3szFcMZm5BCRkyUYvZr?=
 =?us-ascii?Q?Uhs6B9xIbDprYICmE/UUHMzKi45d8JDrL/g8DDqffW+b/+BaHgb7WIIh0Fn6?=
 =?us-ascii?Q?ZKyARNzACBBg/Q0ICzHhpGhthogl+Cn3Q+B7U7OMovzirrUFQjTd96YWdItL?=
 =?us-ascii?Q?I1z8hMbR5+1dy7HvEssSh+lwMkb2ya4zorSLpl8Idy2EOAum7lNDI47ppm5n?=
 =?us-ascii?Q?Sk9m76a0epQDvWWJI0JVwyGI20os0ciyDGoJtV/A8BesYt+Nkomvkb+rOHZH?=
 =?us-ascii?Q?v2A/3+Kahu6q3DZOrb+IbiN38vLx3kw2lTKH8wp4VWrxlo0/Sn+OpNo8gd36?=
 =?us-ascii?Q?P7Q0O037EiCgFeiEzjHlRU52kvKnW5TNe67TnOvayvyxxeIHql9P2c4W7ZQA?=
 =?us-ascii?Q?6y6nUy1KU3f7gi8SuB5QPgBpmyxpuj13SFeUECq5uWd39GlLXTE6IHY8T3xz?=
 =?us-ascii?Q?0+zDBah7fYdJC3zXmaEMkupmi20F0J+RXNiWhQEYTSvNEm7PRyt73HvGzS/W?=
 =?us-ascii?Q?buyw8rL9lQG2qRN8Bg3wQuZg4J/TJ5o46l8SMxFtF9y0CefUV/5/hQ0EJwGi?=
 =?us-ascii?Q?ZHzWtt43HjxuyIV6tft9cRMaXesJEFrK1JqLhzh8Fkz5PmSN8SSnOWjgje3H?=
 =?us-ascii?Q?w7RzZYwcWi7znzt9Cfpx9dQasJ2VZ78PLVi1mlhehJJFt12DQte65tj9N6VR?=
 =?us-ascii?Q?AjSHlery8P8w0LqrY7R06i5on/NwumQdWURkcoBqO8G3AqhhbM+UKxiB0hXe?=
 =?us-ascii?Q?NfghX0pTKtRz1GPxWCO7zuksGwna8pmAQf50VHrEdy18Aum8b1Qf1PWYdE6U?=
 =?us-ascii?Q?AH6Yu12BrVzIt0/QC1raihIBd/AP14NcZKpuaPG80ctwvpnWj7DvfNs3dtzn?=
 =?us-ascii?Q?NTZeX3A2tCm6TYbmHOUXHzvkKX9m0eI6hnQmh9SGX+nb3Xy8xqPEDKXV0wo8?=
 =?us-ascii?Q?tr5AZvudM8vOCybjvT+ejmiksZbCSealRGvMp4QVpmxRFPjc1XqnttyCeHv3?=
 =?us-ascii?Q?x/6O/4rRhiyQCNfj7BePHp4EaQUMEC76BLj6Mm+PHZ43rl+k1zmPyFBo5gkm?=
 =?us-ascii?Q?qSS/fMvmcW8XFSmGsFfxim9fW382JZ9BA8o7oym6LU3/lYeIekOqFgN72kAj?=
 =?us-ascii?Q?miLw7eJ+xth4SKAwsdez3V1+NH52yOa+mJaxq4U2Y2rLqVqmXw+F1OMpoZh3?=
 =?us-ascii?Q?OFHzFc85Bg7m8faYypaQ4OMy/clK9MEFnK8DsdDScHvQA4W7X7BDaEQWhRad?=
 =?us-ascii?Q?AfwndcgJSsUB8qKcmWXz7pHX+SBajM+cE34bHNVuhOPhtf1TFRsncDQX1U6g?=
 =?us-ascii?Q?Byst8ns5PN+X5Pe73YVuAjDlZmdWH0MmjW45KsuLc9civZT40RlHT/B8NNaJ?=
 =?us-ascii?Q?kzoaB7IKuQK2qDEtzkk1YttIvhWqL6ehSMwZHVH8PCfWwnrotjCEjbpGYuxf?=
 =?us-ascii?Q?jE6XYDfAiEtnw/h97adgD2/bRo1zsJ+MF8k4bB6kIy23lSkdmzqkHXuVG77N?=
 =?us-ascii?Q?Jf0pTa2+1eDNwVL1PXTVMMVFlc6MSHS4969qiCJ62UpG6ekSIuEoqhLOtqio?=
 =?us-ascii?Q?+ag8nFNzAS5tpfg/h5kTN0qH42GUl35FN03aTFdK+wABK2jhqS/57AHNzFnm?=
 =?us-ascii?Q?sIlDU3U6BPRgvw1PU7Hz/HM=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5880.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8bcddf62-95d1-4f5e-c346-08d9f73727bc
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Feb 2022 01:44:11.0486
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: RZvvIQe43NRjB+mFPepEaf4oHXd1Pa5WccSuLPQ6BcXMR0dhdvX8s+XitbPEfOJmVfwIMQA3Ru3yjAPS3uhSQg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB2566
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
	DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,RDNS_NONE,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE
	autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Feb 23, 2022 at 05:00:12PM +0100, gregkh@linuxfoundation.org wrote:
> On Wed, Feb 23, 2022 at 09:38:20AM -0500, stern@rowland.harvard.edu wrote=
:
> > Which bus locks are you referring to?  I'm not aware of any locks=20
> > that synchronize dev_uevent() with anything (in particular, with=20
> > driver unbinding).
>=20
> The locks in the driver core that handle the binding and unbinding of=20
> drivers to devices.
>=20
> > And as far as I know, usb_gadget_remove_driver() doesn't play any=20
> > odd tricks with pointers.
>=20
> Ah, I never noticed that this is doing a "fake" bus and does the=20
> bind/unbind itself outside of the driver core.  It should just be a=20
> normal bus type and have the core do the work for it, but oh well.
>=20
> And there is a lock that should serialize all of this already, so it's=20
> odd that this is able to be triggered at all.

>>I guess at a minimum the UDC core should hold the device lock when it reg=
isters, unregisters, binds, or unbinds UDC and gadget devices. =20
>>Would that be enough to fix the problem?  I really don't understand how s=
ysfs file access gets synchronized with device removal.


Agree with you, in usb_gadget_remove_driver() function, the udc->dev.driver=
 and udc->gadget->dev.driver be set to null
without any protection, so when the udevd accessed the dev->driver, this ad=
dress may be invalid at this time.
maybe the operation of dev->driver can be protected by device_lock().=20

Thanks,
Zqiang


> Unless the device is being removed at the same time it was manually=20
> unbound from the driver?  If so, then this really should be fixed up=20
> to use the driver core logic instead...
>>
>>Device removal does of course trigger unbinding, but they always take pla=
ce in the same thread so it isn't an issue.
>>
>>Probably part of the reason people don't want to use the driver core here=
 is so that they can specify which UDC a gadget driver should bind to.  The=
 driver core would always bind each new gadget to the first registered gadg=
et driver.
>>
>>When Dave Brownell originally wrote the gadget subsystem, I believe he di=
dn't bother to integrate it with the driver core because it was a "bus" wit=
h only a single device and a single driver.  The ability to have multiple U=
DCs in the system was added later.
>>
>>Alan Stern