Message ID | E1pCdoY-0044aT-A5@rmk-PC.armlinux.org.uk |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4e01:0:0:0:0:0 with SMTP id p1csp4528761wrt; Tue, 3 Jan 2023 01:54:58 -0800 (PST) X-Google-Smtp-Source: AMrXdXtgOalPgmQNw1KpaZR74dpNlMZYqJNc1bEYHVZVV8aQk25IJg5fha0Gikgxeb7H/9zwdmXR X-Received: by 2002:a05:6a20:7b25:b0:a2:e391:f8f3 with SMTP id s37-20020a056a207b2500b000a2e391f8f3mr59421954pzh.34.1672739698287; Tue, 03 Jan 2023 01:54:58 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1672739698; cv=none; d=google.com; s=arc-20160816; b=uiOcNyOl9RRHTIqnvwxY49eTu8al3QY+yF3+xhmQtxDPVlwPQvJgm+V6AzVr7Ku/sZ uWg/R1FoiSctM1HUN5FXOkaDE+noB4FTHtom9Y6K/aQDSRyZ3OxA3GrZygIRbPhmD51U FCLrfLU2PHBmMm4n9L9xNBzv/V7U5qhkxui3OCTPf9fhVawSN26uUS8N/p2SRsx6G4aE bJsOYzJYEMtKye/CEPkWLM0AICXFCsAJJvySuQ7dI5G7Y6E0x0CKmH3N3rmlT0ZwV4Pt nhE67HnsmIF2iCSeClfxGetJI0wAKUi/iSO+Lqg4BanVC0vidnactAKfo1+WlJ/s6psF YMaw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:date:sender:message-id:content-transfer-encoding :content-disposition:mime-version:subject:cc:to:from:dkim-signature; bh=4fMNHgZDzPHb49xSU4TUscrYEmRUetsr0RmDQrt06rI=; b=TiuzT/Tre67Jl7E3R1w77bDKsZpAuTP5f8fgLMlF3SIxfG25kao3RZrQ/brGbtk55G Id+dOAxe93TRX6JvIQsoNPNKZ3lFBO2J6VCvxPGJC+KGEIGSq2V5WFAqAegr8owW+3L7 Hx5EGOpgRPmyCy+9B7cOTCuPFXXy3PI92kbOgTj/muTMrz7rtbD6/QcsNVpmNJQ9JkiX pGkketg6E1hbICFfOoEL6jCdHEQeiaaV2sDKrOd03NTB07ytdO5CRs93olKyIm2k4YyZ +hO0iu+6O90Ifbog0auWG8D91jln71o4MSsNb7mQPdNl15Yyt7hdV7lfVBD/NxZ4RGJJ 7j2g== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail (test mode) header.i=@armlinux.org.uk header.s=pandora-2019 header.b="mNrk/czo"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=armlinux.org.uk Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id s184-20020a632cc1000000b004a0dd5c1e54si9507278pgs.108.2023.01.03.01.54.44; Tue, 03 Jan 2023 01:54:58 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=fail (test mode) header.i=@armlinux.org.uk header.s=pandora-2019 header.b="mNrk/czo"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=armlinux.org.uk Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237168AbjACJmo (ORCPT <rfc822;tmhikaru@gmail.com> + 99 others); Tue, 3 Jan 2023 04:42:44 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59034 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232970AbjACJmm (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 3 Jan 2023 04:42:42 -0500 Received: from pandora.armlinux.org.uk (pandora.armlinux.org.uk [IPv6:2001:4d48:ad52:32c8:5054:ff:fe00:142]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4918D2D2 for <linux-kernel@vger.kernel.org>; Tue, 3 Jan 2023 01:42:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Date:Sender:Message-Id:Content-Type: Content-Transfer-Encoding:MIME-Version:Subject:Cc:To:From:Reply-To:Content-ID :Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To: Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Id:List-Help: List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=4fMNHgZDzPHb49xSU4TUscrYEmRUetsr0RmDQrt06rI=; b=mNrk/czourWWRaA79ofmXCfNmP /eYQUwcpOXo0e3atgF/tTg29ftLZxbOJl2rjaBotIrtBedMULRz5kaWblLEEZQRRuyRyPgZcWDBdD VgRUF51F4ZgV9vokiZ0WqFLwVnIoWID7wiLPROknCsAM3usgBzslc/TNzUutRrYDnXVjrKiM9FjQo dwU7YhfRQmvreef1WmtQGyseBrCYI0PSiUaW0RQlaUxLAqwatUxxXxqnzYz0vc1bpuPMXFAcd0DcO 2z6ru8pEYVMQ2navBfm8J0vGJyJU0qFOewO5tuvybMZsLnKxXhnLY0hCUWzHz+yycyMiiOeMuFA3C FasNF7TQ==; Received: from e0022681537dd.dyn.armlinux.org.uk ([fd8f:7570:feb6:1:222:68ff:fe15:37dd]:37962 helo=rmk-PC.armlinux.org.uk) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from <rmk@armlinux.org.uk>) id 1pCdoZ-0005AA-88; Tue, 03 Jan 2023 09:42:34 +0000 Received: from rmk by rmk-PC.armlinux.org.uk with local (Exim 4.94.2) (envelope-from <rmk@rmk-PC.armlinux.org.uk>) id 1pCdoY-0044aT-A5; Tue, 03 Jan 2023 09:42:34 +0000 From: "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk> To: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH] nvmem: fix registration vs use race MIME-Version: 1.0 Content-Disposition: inline Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="utf-8" Message-Id: <E1pCdoY-0044aT-A5@rmk-PC.armlinux.org.uk> Sender: Russell King <rmk@armlinux.org.uk> Date: Tue, 03 Jan 2023 09:42:34 +0000 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_NONE autolearn=ham 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 X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1753994702290569762?= X-GMAIL-MSGID: =?utf-8?q?1753994702290569762?= |
Series |
nvmem: fix registration vs use race
|
|
Commit Message
Russell King (Oracle)
Jan. 3, 2023, 9:42 a.m. UTC
The i.MX6 CPU frequency driver sometimes fails to register at boot time
due to nvmem_cell_read_u32() sporadically returning -ENOENT.
This happens because there is a window where __nvmem_device_get() in
of_nvmem_cell_get() is able to return the nvmem device, but as cells
have been setup, nvmem_find_cell_entry_by_node() returns NULL.
The occurs because the nvmem core registration code violates one of the
fundamental principles of kernel programming: do not publish data
structures before their setup is complete.
Fix this by making nvmem core code conform with this principle.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/nvmem/core.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
Comments
On 03/01/2023 09:42, Russell King (Oracle) wrote: > The i.MX6 CPU frequency driver sometimes fails to register at boot time > due to nvmem_cell_read_u32() sporadically returning -ENOENT. > > This happens because there is a window where __nvmem_device_get() in > of_nvmem_cell_get() is able to return the nvmem device, but as cells > have been setup, nvmem_find_cell_entry_by_node() returns NULL. > > The occurs because the nvmem core registration code violates one of the > fundamental principles of kernel programming: do not publish data > structures before their setup is complete. > > Fix this by making nvmem core code conform with this principle. > how about a Fixes tag and Cc stable? > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > --- > drivers/nvmem/core.c | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c > index 321d7d63e068..6b89fb6fa582 100644 > --- a/drivers/nvmem/core.c > +++ b/drivers/nvmem/core.c > @@ -835,22 +835,16 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) > nvmem->dev.groups = nvmem_dev_groups; > #endif > > - dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name); > - > - rval = device_register(&nvmem->dev); > - if (rval) > - goto err_put_device; > - > if (nvmem->nkeepout) { > rval = nvmem_validate_keepouts(nvmem); > if (rval) > - goto err_device_del; > + goto err_put_device; AFAIU, as we never did a get_device/kobject_get, calling put_device at this point will not invoke a release callback which can potentially leak both nvmem and ida. --srini > } > > if (config->compat) { > rval = nvmem_sysfs_setup_compat(nvmem, config); > if (rval) > - goto err_device_del; > + goto err_put_device; > } > > if (config->cells) { > @@ -867,6 +861,12 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) > if (rval) > goto err_remove_cells; > > + dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name); > + > + rval = device_register(&nvmem->dev); > + if (rval) > + goto err_remove_cells; > + > blocking_notifier_call_chain(&nvmem_notifier, NVMEM_ADD, nvmem); > > return nvmem; > @@ -876,8 +876,6 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) > err_teardown_compat: > if (config->compat) > nvmem_sysfs_remove_compat(nvmem, config); > -err_device_del: > - device_del(&nvmem->dev); > err_put_device: > put_device(&nvmem->dev); >
On Tue, Jan 03, 2023 at 11:30:36AM +0000, Srinivas Kandagatla wrote: > > > On 03/01/2023 09:42, Russell King (Oracle) wrote: > > The i.MX6 CPU frequency driver sometimes fails to register at boot time > > due to nvmem_cell_read_u32() sporadically returning -ENOENT. > > > > This happens because there is a window where __nvmem_device_get() in > > of_nvmem_cell_get() is able to return the nvmem device, but as cells > > have been setup, nvmem_find_cell_entry_by_node() returns NULL. > > > > The occurs because the nvmem core registration code violates one of the > > fundamental principles of kernel programming: do not publish data > > structures before their setup is complete. > > > > Fix this by making nvmem core code conform with this principle. > > > how about a Fixes tag and Cc stable? Which commit do you suggest? This error goes all the way back to the inception of nvmem, commit eace75cfdcf7 ("nvmem: Add a simple NVMEM framework for nvmem providers") but clearly its going to be a lot of effort to backport it all the way due to all the changes.
On 03/01/2023 11:46, Russell King (Oracle) wrote: > On Tue, Jan 03, 2023 at 11:30:36AM +0000, Srinivas Kandagatla wrote: >> >> >> On 03/01/2023 09:42, Russell King (Oracle) wrote: >>> The i.MX6 CPU frequency driver sometimes fails to register at boot time >>> due to nvmem_cell_read_u32() sporadically returning -ENOENT. >>> >>> This happens because there is a window where __nvmem_device_get() in >>> of_nvmem_cell_get() is able to return the nvmem device, but as cells >>> have been setup, nvmem_find_cell_entry_by_node() returns NULL. >>> >>> The occurs because the nvmem core registration code violates one of the >>> fundamental principles of kernel programming: do not publish data >>> structures before their setup is complete. >>> >>> Fix this by making nvmem core code conform with this principle. >>> >> how about a Fixes tag and Cc stable? > > Which commit do you suggest? This error goes all the way back to the > inception of nvmem, commit > > eace75cfdcf7 ("nvmem: Add a simple NVMEM framework for nvmem providers") > > but clearly its going to be a lot of effort to backport it all the > way due to all the changes. I understand the backport issue, On the other hand as this a real issue backporting to atleast stable kernels would be worth. --srini >
On Tue, Jan 03, 2023 at 12:42:49PM +0000, Srinivas Kandagatla wrote: > > > On 03/01/2023 11:46, Russell King (Oracle) wrote: > > On Tue, Jan 03, 2023 at 11:30:36AM +0000, Srinivas Kandagatla wrote: > > > > > > > > > On 03/01/2023 09:42, Russell King (Oracle) wrote: > > > > The i.MX6 CPU frequency driver sometimes fails to register at boot time > > > > due to nvmem_cell_read_u32() sporadically returning -ENOENT. > > > > > > > > This happens because there is a window where __nvmem_device_get() in > > > > of_nvmem_cell_get() is able to return the nvmem device, but as cells > > > > have been setup, nvmem_find_cell_entry_by_node() returns NULL. > > > > > > > > The occurs because the nvmem core registration code violates one of the > > > > fundamental principles of kernel programming: do not publish data > > > > structures before their setup is complete. > > > > > > > > Fix this by making nvmem core code conform with this principle. > > > > > > > how about a Fixes tag and Cc stable? > > > > Which commit do you suggest? This error goes all the way back to the > > inception of nvmem, commit > > > > eace75cfdcf7 ("nvmem: Add a simple NVMEM framework for nvmem providers") > > > > but clearly its going to be a lot of effort to backport it all the > > way due to all the changes. > > I understand the backport issue, On the other hand as this a real issue > backporting to atleast stable kernels would be worth. I'll add this commit as a fixes tag, but I don't have the ability to test backports of this, since the use of nvmem on imx6 platforms is relatively recent. How do you suggest we end up with tested backports for stable trees?
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c index 321d7d63e068..6b89fb6fa582 100644 --- a/drivers/nvmem/core.c +++ b/drivers/nvmem/core.c @@ -835,22 +835,16 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) nvmem->dev.groups = nvmem_dev_groups; #endif - dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name); - - rval = device_register(&nvmem->dev); - if (rval) - goto err_put_device; - if (nvmem->nkeepout) { rval = nvmem_validate_keepouts(nvmem); if (rval) - goto err_device_del; + goto err_put_device; } if (config->compat) { rval = nvmem_sysfs_setup_compat(nvmem, config); if (rval) - goto err_device_del; + goto err_put_device; } if (config->cells) { @@ -867,6 +861,12 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) if (rval) goto err_remove_cells; + dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name); + + rval = device_register(&nvmem->dev); + if (rval) + goto err_remove_cells; + blocking_notifier_call_chain(&nvmem_notifier, NVMEM_ADD, nvmem); return nvmem; @@ -876,8 +876,6 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) err_teardown_compat: if (config->compat) nvmem_sysfs_remove_compat(nvmem, config); -err_device_del: - device_del(&nvmem->dev); err_put_device: put_device(&nvmem->dev);