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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E3315C433F5 for ; Wed, 23 Feb 2022 16:03:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242585AbiBWQEO (ORCPT ); Wed, 23 Feb 2022 11:04:14 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57410 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242589AbiBWQEO (ORCPT ); Wed, 23 Feb 2022 11:04:14 -0500 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4107AC1149; Wed, 23 Feb 2022 08:03:46 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 02341B81E07; Wed, 23 Feb 2022 16:03:45 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0674DC340E7; Wed, 23 Feb 2022 16:03:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1645632223; bh=jWeqBXnmhv/yp6ESTH9OA8NuBmEWEyuUdhFPVPzAZGc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Hdc0lrA5dZRtIeA1RbNiQdoue/GFBd1uExyV5sEiImDGn9g8/0gnRnDq0hvxi4n2r v9O0f1M7TmK0c8lx+ozjSuDDz9e8c1PY0MtvWUMMlDY8fxkcE+//9igW8MESSZn74/ uRYM/PUkLjQMaLa4jGCskJB1lUymU+lOib7KSWxA= Date: Wed, 23 Feb 2022 17:03:40 +0100 From: Greg Kroah-Hartman To: Jason Gunthorpe Cc: Robin Murphy , Lu Baolu , Christoph Hellwig , Joerg Roedel , Alex Williamson , Bjorn Helgaas , Kevin Tian , Ashok Raj , kvm@vger.kernel.org, rafael@kernel.org, David Airlie , linux-pci@vger.kernel.org, Thierry Reding , Diana Craciun , Dmitry Osipenko , Will Deacon , Stuart Yoder , Jonathan Hunter , Chaitanya Kulkarni , Dan Williams , Cornelia Huck , linux-kernel@vger.kernel.org, Li Yang , iommu@lists.linux-foundation.org, Jacob jun Pan , Daniel Vetter Subject: Re: [PATCH v6 02/11] driver core: Add dma_cleanup callback in bus_type Message-ID: References: <1acb8748-8d44-688d-2380-f39ec820776f@arm.com> <20220222151632.GB10061@nvidia.com> <3d4c3bf1-fed6-f640-dc20-36d667de7461@arm.com> <20220222235353.GF10061@nvidia.com> <171bec90-5ea6-b35b-f027-1b5e961f5ddf@linux.intel.com> <880a269d-d39d-bab3-8d19-b493e874ec99@arm.com> <20220223134627.GO10061@nvidia.com> <20220223140901.GP10061@nvidia.com> <20220223143011.GQ10061@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220223143011.GQ10061@nvidia.com> Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Wed, Feb 23, 2022 at 10:30:11AM -0400, Jason Gunthorpe wrote: > On Wed, Feb 23, 2022 at 10:09:01AM -0400, Jason Gunthorpe wrote: > > On Wed, Feb 23, 2022 at 03:06:35PM +0100, Greg Kroah-Hartman wrote: > > > On Wed, Feb 23, 2022 at 09:46:27AM -0400, Jason Gunthorpe wrote: > > > > On Wed, Feb 23, 2022 at 01:04:00PM +0000, Robin Murphy wrote: > > > > > > > > > 1 - tmp->driver is non-NULL because tmp is already bound. > > > > > 1.a - If tmp->driver->driver_managed_dma == 0, the group must currently be > > > > > DMA-API-owned as a whole. Regardless of what driver dev has unbound from, > > > > > its removal does not release someone else's DMA API (co-)ownership. > > > > > > > > This is an uncommon locking pattern, but it does work. It relies on > > > > the mutex being an effective synchronization barrier for an unlocked > > > > store: > > > > > > > > WRITE_ONCE(dev->driver, NULL) > > > > > > Only the driver core should be messing with the dev->driver pointer as > > > when it does so, it already has the proper locks held. Do I need to > > > move that to a "private" location so that nothing outside of the driver > > > core can mess with it? > > > > It would be nice, I've seen a abuse and mislocking of it in drivers > > Though to be clear, what Robin is describing is still keeping the > dev->driver stores in dd.c, just reading it in a lockless way from > other modules. "other modules" should never care if a device has a driver bound to it because instantly after the check happens, it can change so what ever logic it wanted to do with that knowledge is gone. Unless the bus lock is held that the device is on, but that should be only accessable from within the driver core as it controls that type of stuff, not any random other part of the kernel. And in looking at this, ick, there are loads of places in the kernel that are thinking that this pointer being set to something actually means something. Sometimes it does, but lots of places, it doesn't as it can change. In a semi-related incident right now, we currently have a syzbot failure in the usb gadget code where it was manipulating the ->driver pointer directly and other parts of the kernel are crashing. See https://lore.kernel.org/r/PH0PR11MB58805E3C4CF7D4C41D49BFCFDA3C9@PH0PR11MB5880.namprd11.prod.outlook.com for the thread. I'll poke at this as a background task to try to clean up over time. thanks, greg k-h