From patchwork Tue Feb 18 02:28:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: srinivas pandruvada X-Patchwork-Id: 1195764 Return-Path: Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BBB34C34031 for ; Tue, 18 Feb 2020 02:29:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9BCFE222D9 for ; Tue, 18 Feb 2020 02:29:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726438AbgBRC2u (ORCPT ); Mon, 17 Feb 2020 21:28:50 -0500 Received: from mga09.intel.com ([134.134.136.24]:32442 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726212AbgBRC2t (ORCPT ); Mon, 17 Feb 2020 21:28:49 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Feb 2020 18:28:48 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,454,1574150400"; d="scan'208";a="348602155" Received: from spandruv-desk.jf.intel.com ([10.54.75.21]) by fmsmga001.fm.intel.com with ESMTP; 17 Feb 2020 18:28:47 -0800 From: Srinivas Pandruvada To: dvhart@infradead.org, andy@infradead.org Cc: platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org, andriy.shevchenko@linux.intel.com, Srinivas Pandruvada , Dan Carpenter Subject: [REPOST][PATCH 1/2] platform/x86/intel-uncore-freq: Fix static checker issue and potential race condition Date: Mon, 17 Feb 2020 18:28:43 -0800 Message-Id: <20200218022844.179988-2-srinivas.pandruvada@linux.intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200218022844.179988-1-srinivas.pandruvada@linux.intel.com> References: <20200218022844.179988-1-srinivas.pandruvada@linux.intel.com> MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org There is a possible race condition when: All CPUs in a package is offlined and just before the last CPU offline, user tries to read sysfs entry and read happens while offline callback is about to delete the sysfs entry. Although not reproduced but this is possible scenerio and can be reproduced by adding a msleep() in the show_min_max_freq_khz() before mutex_lock() and read min_freq attribute from user space. Before msleep() finishes, force every CPUs in a package offline. This will cause deadlock, with offline and sysfs read/write operation because of mutex_lock. The uncore_remove_die_entry() will not release mutex till read/write callback returns because of kobject_put() and read/write callback waiting on mutex. We don't have to remove the sysfs folder when the package is offline. While there is no CPU present, we can fail the read/write calls by returning ENXIO error. So remove the kobject_put() call in offline path. This also address the warning from static checker, as there is no access to "data" variable after kobject_put: "The patch 49a474c7ba51: "platform/x86: Add support for Uncore frequency control" from Jan 13, 2020, leads to the following static checker warning: drivers/platform/x86/intel-uncore-frequency.c:285 uncore_remove_die_entry() error: dereferencing freed memory 'data' " Reported-by: Dan Carpenter Signed-off-by: Srinivas Pandruvada --- drivers/platform/x86/intel-uncore-frequency.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/platform/x86/intel-uncore-frequency.c b/drivers/platform/x86/intel-uncore-frequency.c index 2b1a0734c3f8..c83ec95e8f3e 100644 --- a/drivers/platform/x86/intel-uncore-frequency.c +++ b/drivers/platform/x86/intel-uncore-frequency.c @@ -97,6 +97,9 @@ static int uncore_read_ratio(struct uncore_data *data, unsigned int *min, u64 cap; int ret; + if (data->control_cpu < 0) + return -ENXIO; + ret = rdmsrl_on_cpu(data->control_cpu, MSR_UNCORE_RATIO_LIMIT, &cap); if (ret) return ret; @@ -116,6 +119,11 @@ static int uncore_write_ratio(struct uncore_data *data, unsigned int input, mutex_lock(&uncore_lock); + if (data->control_cpu < 0) { + ret = -ENXIO; + goto finish_write; + } + input /= UNCORE_FREQ_KHZ_MULTIPLIER; if (!input || input > 0x7F) { ret = -EINVAL; @@ -273,18 +281,15 @@ static void uncore_add_die_entry(int cpu) mutex_unlock(&uncore_lock); } -/* Last CPU in this die is offline, so remove sysfs entries */ +/* Last CPU in this die is offline, make control cpu invalid */ static void uncore_remove_die_entry(int cpu) { struct uncore_data *data; mutex_lock(&uncore_lock); data = uncore_get_instance(cpu); - if (data) { - kobject_put(&data->kobj); + if (data) data->control_cpu = -1; - data->valid = false; - } mutex_unlock(&uncore_lock); } From patchwork Tue Feb 18 02:28:44 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: srinivas pandruvada X-Patchwork-Id: 1195765 Return-Path: Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A44CCC34022 for ; Tue, 18 Feb 2020 02:29:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7DAF321D7D for ; Tue, 18 Feb 2020 02:29:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726402AbgBRC2u (ORCPT ); Mon, 17 Feb 2020 21:28:50 -0500 Received: from mga09.intel.com ([134.134.136.24]:32442 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726171AbgBRC2t (ORCPT ); Mon, 17 Feb 2020 21:28:49 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Feb 2020 18:28:48 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,454,1574150400"; d="scan'208";a="348602156" Received: from spandruv-desk.jf.intel.com ([10.54.75.21]) by fmsmga001.fm.intel.com with ESMTP; 17 Feb 2020 18:28:47 -0800 From: Srinivas Pandruvada To: dvhart@infradead.org, andy@infradead.org Cc: platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org, andriy.shevchenko@linux.intel.com, Srinivas Pandruvada Subject: [REPOST][PATCH 2/2] platform/x86/intel-uncore-freq: Add release callback Date: Mon, 17 Feb 2020 18:28:44 -0800 Message-Id: <20200218022844.179988-3-srinivas.pandruvada@linux.intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200218022844.179988-1-srinivas.pandruvada@linux.intel.com> References: <20200218022844.179988-1-srinivas.pandruvada@linux.intel.com> MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On module unload wait for relese callback for each packag_die entry and then free the memory. This is done by waiting on a completion object, till release() callback. While here, also change to kobject_init_and_add() to kobject_create_and_add() to simplify. Signed-off-by: Srinivas Pandruvada --- drivers/platform/x86/intel-uncore-frequency.c | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/drivers/platform/x86/intel-uncore-frequency.c b/drivers/platform/x86/intel-uncore-frequency.c index c83ec95e8f3e..82f2de7c4112 100644 --- a/drivers/platform/x86/intel-uncore-frequency.c +++ b/drivers/platform/x86/intel-uncore-frequency.c @@ -38,6 +38,7 @@ */ struct uncore_data { struct kobject kobj; + struct completion kobj_unregister; u64 stored_uncore_data; u32 initial_min_freq_khz; u32 initial_max_freq_khz; @@ -52,7 +53,7 @@ static int uncore_max_entries __read_mostly; /* Storage for uncore data for all instances */ static struct uncore_data *uncore_instances; /* Root of the all uncore sysfs kobjs */ -struct kobject uncore_root_kobj; +struct kobject *uncore_root_kobj; /* Stores the CPU mask of the target CPUs to use during uncore read/write */ static cpumask_t uncore_cpu_mask; /* CPU online callback register instance */ @@ -225,15 +226,19 @@ static struct attribute *uncore_attrs[] = { NULL }; +static void uncore_sysfs_entry_release(struct kobject *kobj) +{ + struct uncore_data *data = to_uncore_data(kobj); + + complete(&data->kobj_unregister); +} + static struct kobj_type uncore_ktype = { + .release = uncore_sysfs_entry_release, .sysfs_ops = &kobj_sysfs_ops, .default_attrs = uncore_attrs, }; -static struct kobj_type uncore_root_ktype = { - .sysfs_ops = &kobj_sysfs_ops, -}; - /* Caller provides protection */ static struct uncore_data *uncore_get_instance(unsigned int cpu) { @@ -271,8 +276,10 @@ static void uncore_add_die_entry(int cpu) uncore_read_ratio(data, &data->initial_min_freq_khz, &data->initial_max_freq_khz); + init_completion(&data->kobj_unregister); + ret = kobject_init_and_add(&data->kobj, &uncore_ktype, - &uncore_root_kobj, str); + uncore_root_kobj, str); if (!ret) { data->control_cpu = cpu; data->valid = true; @@ -391,11 +398,12 @@ static int __init intel_uncore_init(void) if (!uncore_instances) return -ENOMEM; - ret = kobject_init_and_add(&uncore_root_kobj, &uncore_root_ktype, - &cpu_subsys.dev_root->kobj, - "intel_uncore_frequency"); - if (ret) + uncore_root_kobj = kobject_create_and_add("intel_uncore_frequency", + &cpu_subsys.dev_root->kobj); + if (!uncore_root_kobj) { + ret = -ENOMEM; goto err_free; + } ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "platform/x86/uncore-freq:online", @@ -415,7 +423,7 @@ static int __init intel_uncore_init(void) err_rem_state: cpuhp_remove_state(uncore_hp_state); err_rem_kobj: - kobject_put(&uncore_root_kobj); + kobject_put(uncore_root_kobj); err_free: kfree(uncore_instances); @@ -430,10 +438,12 @@ static void __exit intel_uncore_exit(void) unregister_pm_notifier(&uncore_pm_nb); cpuhp_remove_state(uncore_hp_state); for (i = 0; i < uncore_max_entries; ++i) { - if (uncore_instances[i].valid) + if (uncore_instances[i].valid) { kobject_put(&uncore_instances[i].kobj); + wait_for_completion(&uncore_instances[i].kobj_unregister); + } } - kobject_put(&uncore_root_kobj); + kobject_put(uncore_root_kobj); kfree(uncore_instances); } module_exit(intel_uncore_exit)