From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752165AbdBTEXE (ORCPT ); Sun, 19 Feb 2017 23:23:04 -0500 Received: from ale.deltatee.com ([207.54.116.67]:48263 "EHLO ale.deltatee.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751875AbdBTEXC (ORCPT ); Sun, 19 Feb 2017 23:23:02 -0500 To: Dan Williams References: <20170210145124.GA24594@kroah.com> <1486749440-24309-1-git-send-email-logang@deltatee.com> <20e55da4-b3c1-7c57-15cf-b0aa52442e18@deltatee.com> Cc: Keith Busch , Myron Stowe , Greg Kroah-Hartman , Bjorn Helgaas , Geert Uytterhoeven , Jonathan Corbet , "David S. Miller" , Andrew Morton , Emil Velikov , Mauro Carvalho Chehab , Guenter Roeck , Jarkko Sakkinen , Linus Walleij , Ryusuke Konishi , Stefan Berger , Wei Zhang , linux-doc@vger.kernel.org, linux-pci@vger.kernel.org, Linux Kernel Mailing List , linux-nvme@lists.infradead.org, Stephen Bates , Kurt Schwemmer , Jason Gunthorpe From: Logan Gunthorpe Message-ID: <2cd5df88-c018-3e76-fd08-483fec374932@deltatee.com> Date: Sun, 19 Feb 2017 21:22:35 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 50.66.97.235 X-SA-Exim-Rcpt-To: jgunthorpe@obsidianresearch.com, kurt.schwemmer@microsemi.com, stephen.bates@microsemi.com, linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, linux-doc@vger.kernel.org, wzhang@fb.com, stefanb@linux.vnet.ibm.com, konishi.ryusuke@lab.ntt.co.jp, linus.walleij@linaro.org, jarkko.sakkinen@linux.intel.com, linux@roeck-us.net, mchehab@kernel.org, emil.l.velikov@gmail.com, akpm@linux-foundation.org, davem@davemloft.net, corbet@lwn.net, geert+renesas@glider.be, bhelgaas@google.com, gregkh@linuxfoundation.org, myron.stowe@gmail.com, keith.busch@intel.com, dan.j.williams@intel.com X-SA-Exim-Mail-From: logang@deltatee.com Subject: Re: [PATCH] switchtec: cleanup cdev init X-SA-Exim-Version: 4.2.1 (built Mon, 26 Dec 2011 16:24:06 +0000) X-SA-Exim-Scanned: Yes (on ale.deltatee.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 19/02/17 02:43 PM, Dan Williams wrote: > Is this race present for all file operations? I've only seen it with > mmap() and late faults. So if these other drivers do not support mmap > it's not clear they can trigger the failure. I don't see how it's related to mmap only. I think there's a number of variations on this but the race I see happens if you try to take a reference to the device in the open/close handlers of a char device file. If a driver puts the last remaining reference in the release handler, then the device structure will be freed, and on returning from the release handler, the char_dev code will try to put the last reference to the cdev structure which may have already been free'd. There needs to be a way for the cdev structure to take a reference on the device structure so it doesn't get freed and the kobj.parent trick seems to accomplish this fairly well. Really, in any situation where there's a cdev and a device in the same structure, the life cycles of the two become linked but their reference counts are not and that is the problem here. I feel like a number of developers have made the same realization independently so this would probably be a good thing to clean up. See these commits for examples spanning around 5 years: 4a215aa Input: fix use-after-free introduced with dynamic minor changes 0d5b7da iio: Prevent race between IIO chardev opening and IIO device ba0ef85 tpm: Fix initialization of the cdev Also, seems both my brother and I have independently looked into this exact issue: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg25692.html So, I think it would be a good idea to clean it up with Dan's proposed API cleanup. If I have some time tomorrow, I may throw together a patch set with that patch and the changes to the instances around the kernel. Logan