From patchwork Tue Oct 17 01:17:51 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Frank Rowand X-Patchwork-Id: 10010965 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id DF04660211 for ; Tue, 17 Oct 2017 07:35:40 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CF3DB286C2 for ; Tue, 17 Oct 2017 07:35:40 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C404628756; Tue, 17 Oct 2017 07:35:40 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 49E87286C2 for ; Tue, 17 Oct 2017 07:35:40 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id DC5986E5A4; Tue, 17 Oct 2017 07:35:22 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-pg0-x243.google.com (mail-pg0-x243.google.com [IPv6:2607:f8b0:400e:c05::243]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0CDE46E571 for ; Tue, 17 Oct 2017 01:18:18 +0000 (UTC) Received: by mail-pg0-x243.google.com with SMTP id s2so115921pge.10 for ; Mon, 16 Oct 2017 18:18:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=itDDbf99PdYlS6TkRUhuPteBe7rfJn3Yy/41d9M9/Lo=; b=f0lxBh1zG7z9B3fRxujJqiSo0G7HtR7JzsP2Efk/Kls0zoGbx+LkKLMqAoHS1EQQHl cTAoF83kx7kZ8TemQvenA4i2af283eON1ajHcSeWTbTKcANXX5w6xDNLvUBvtpvLkEsO 4GHcc+eqTVPGK45EYrJcjJU7AQY5uVMHeOTRRMSYzmxYUbUOxn0NFF11KUlZ2UIG1o3x kDEnKFqVTTvi5VJQOyBdJ4hfgbKwPZ4VJVtOU7xZVgvD1U9bYLy+XOmzBEqNcZuqa3D9 dLip7x3tVF38OpevH4eazAP0fxbLVZFhSFvLKyI+87oZUA9AlMF8szOX3ohuBfs62tAR f6rQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=itDDbf99PdYlS6TkRUhuPteBe7rfJn3Yy/41d9M9/Lo=; b=dR7wcOz1DpM+b8KoxQGIeSvcIj1Ewe095ROqdRJt84ZFQLOBhpRfhxLh/qQXYhmrlV ZnH4hbXcLnjHRULDUWjisaZVLoCnQU00UMhfLilc7CK1tA+xirCntKXsb7v/RIrQs9Ll FhbnSa4q/u+J5cV42xARe3LK58XXceTGqkRTtoJ2DkYCdQyeA71pX4CPaZSSseGJA8xk RZrK3W1X2l+XMA3lpptnmTDC+3bKW6njxCQnvxGTvVkuGWXlcbNAZI5cS/94hzq6XzRS dqJ1BJDlXZFSTg1t4/TUQc088YJbTBzkJ4epI4D6TvqaVG6xz075ek1n+A6MxmsZoB+e mFPw== X-Gm-Message-State: AMCzsaW5aNG7QoOYLaBHbHG8qaEHqAtSuFhqZ3ZeyhR6Q3kVvVZCKJsN VFlrL6VvPvq6pqllQH0etNo= X-Google-Smtp-Source: AOwi7QC+oSiS84m9k7xT3a0J1MVVVrJXIsTLkVXWMNH0ZLxC4IPz5xvmcOl4lSlY+MBFF/WFizjV6Q== X-Received: by 10.159.249.68 with SMTP id h4mr10397498pls.165.1508203097673; Mon, 16 Oct 2017 18:18:17 -0700 (PDT) Received: from localhost.localdomain (c-73-93-215-6.hsd1.ca.comcast.net. [73.93.215.6]) by smtp.gmail.com with ESMTPSA id f1sm16874283pfe.150.2017.10.16.18.18.16 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 16 Oct 2017 18:18:17 -0700 (PDT) From: frowand.list@gmail.com To: Rob Herring , Pantelis Antoniou , David Airlie , Jyri Sarha Subject: [PATCH v2 09/12] of: overlay: avoid race condition between applying multiple overlays Date: Mon, 16 Oct 2017 18:17:51 -0700 Message-Id: <1508203074-26917-10-git-send-email-frowand.list@gmail.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1508203074-26917-1-git-send-email-frowand.list@gmail.com> References: <1508203074-26917-1-git-send-email-frowand.list@gmail.com> X-Mailman-Approved-At: Tue, 17 Oct 2017 07:35:19 +0000 Cc: Mark Rutland , devicetree@vger.kernel.org, Tomi Valkeinen , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP From: Frank Rowand The process of applying an overlay consists of: - unflatten an overlay FDT (flattened device tree) into an EDT (expanded device tree) - fixup the phandle values in the overlay EDT to fit in a range above the phandle values in the live device tree - create the overlay changeset to reflect the contents of the overlay EDT - apply the overlay changeset, to modify the live device tree, potentially changing the maximum phandle value in the live device tree There is currently no protection against two overlay applies concurrently determining what range of phandle values are in use in the live device tree, and subsequently changing that range. Add a mutex to prevent multiple overlay applies from occurring simultaneously. Move of_resolve_phandles() into of_overlay_apply() so that it does not have to be duplicated by each caller of of_overlay_apply(). The test in of_resolve_phandles() that the overlay tree is detached is temporarily disabled so that old style overlay unittests do not fail. Signed-off-by: Frank Rowand --- drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c | 6 ------ drivers/of/of_private.h | 12 +++++++++++ drivers/of/overlay.c | 32 ++++++++++++++++++++++++++++ drivers/of/resolver.c | 7 ++++++ drivers/of/unittest.c | 22 +++++++++++++------ include/linux/of.h | 3 --- 6 files changed, 67 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c b/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c index 7a7be0515bfd..54025af534d4 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c @@ -145,7 +145,6 @@ static struct device_node * __init tilcdc_get_overlay(struct kfree_table *kft) __dtb_tilcdc_slave_compat_begin; static void *overlay_data; struct device_node *overlay; - int ret; if (!size) { pr_warn("%s: No overlay data\n", __func__); @@ -164,11 +163,6 @@ static struct device_node * __init tilcdc_get_overlay(struct kfree_table *kft) } of_node_set_flag(overlay, OF_DETACHED); - ret = of_resolve_phandles(overlay); - if (ret) { - pr_err("%s: Failed to resolve phandles: %d\n", __func__, ret); - return NULL; - } return overlay; } diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index b66e8a812147..03772bdbd94b 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -59,6 +59,18 @@ static inline int of_property_notify(int action, struct device_node *np, } #endif /* CONFIG_OF_DYNAMIC */ +#if defined(CONFIG_OF_RESOLVE) +int of_resolve_phandles(struct device_node *tree); +#endif + +#if defined(CONFIG_OF_OVERLAY) +void of_overlay_mutex_lock(void); +void of_overlay_mutex_unlock(void); +#else +static inline void of_overlay_mutex_lock(void) {}; +static inline void of_overlay_mutex_unlock(void) {}; +#endif + #if defined(CONFIG_OF_UNITTEST) && defined(CONFIG_OF_OVERLAY) extern void __init unittest_unflatten_overlay_base(void); #else diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index a0d3222febdc..b1082c6f7b2c 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -71,6 +71,28 @@ static int build_changeset_next_level(struct overlay_changeset *ovcs, const struct device_node *overlay_node, bool is_symbols_node); +/* + * of_resolve_phandles() finds the largest phandle in the live tree. + * of_overlay_apply() may add a larger phandle to the live tree. + * Do not allow race between two overlays being applied simultaneously: + * mutex_lock(&of_overlay_phandle_mutex) + * of_resolve_phandles() + * of_overlay_apply() + * mutex_unlock(&of_overlay_phandle_mutex) + */ +static DEFINE_MUTEX(of_overlay_phandle_mutex); + +void of_overlay_mutex_lock(void) +{ + mutex_lock(&of_overlay_phandle_mutex); +} + +void of_overlay_mutex_unlock(void) +{ + mutex_unlock(&of_overlay_phandle_mutex); +} + + static LIST_HEAD(ovcs_list); static DEFINE_IDR(ovcs_idr); @@ -615,6 +637,12 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id) goto out; } + of_overlay_mutex_lock(); + + ret = of_resolve_phandles(tree); + if (ret) + goto err_overlay_unlock; + mutex_lock(&of_mutex); ret = init_overlay_changeset(ovcs, tree); @@ -660,9 +688,13 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id) } mutex_unlock(&of_mutex); + of_overlay_mutex_unlock(); goto out; +err_overlay_unlock: + of_overlay_mutex_unlock(); + err_free_overlay_changeset: free_overlay_changeset(ovcs); diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c index 99309cb7d372..8a0f52cfe160 100644 --- a/drivers/of/resolver.c +++ b/drivers/of/resolver.c @@ -275,11 +275,18 @@ int of_resolve_phandles(struct device_node *overlay) err = -EINVAL; goto out; } + +#if 0 + Temporarily disable check so that old style overlay unittests + do not fail when of_resolve_phandles() is moved into + of_overlay_apply(). + if (!of_node_check_flag(overlay, OF_DETACHED)) { pr_err("overlay not detached\n"); err = -EINVAL; goto out; } +#endif phandle_delta = live_tree_max_phandle() + 1; adjust_overlay_phandles(overlay, phandle_delta); diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index db2f170186de..cb9b7674f746 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -994,9 +994,17 @@ static int __init unittest_data_add(void) return -ENODATA; } of_node_set_flag(unittest_data_node, OF_DETACHED); + + /* + * This lock normally encloses of_overlay_apply() as well as + * of_resolve_phandles(). + */ + of_overlay_mutex_lock(); + rc = of_resolve_phandles(unittest_data_node); if (rc) { pr_err("%s: Failed to resolve phandles (rc=%i)\n", __func__, rc); + of_overlay_mutex_unlock(); return -EINVAL; } @@ -1006,6 +1014,7 @@ static int __init unittest_data_add(void) __of_attach_node_sysfs(np); of_aliases = of_find_node_by_path("/aliases"); of_chosen = of_find_node_by_path("/chosen"); + of_overlay_mutex_unlock(); return 0; } @@ -1018,6 +1027,9 @@ static int __init unittest_data_add(void) attach_node_and_children(np); np = next; } + + of_overlay_mutex_unlock(); + return 0; } @@ -2150,16 +2162,11 @@ static int __init overlay_data_add(int onum) } of_node_set_flag(info->np_overlay, OF_DETACHED); - ret = of_resolve_phandles(info->np_overlay); - if (ret) { - pr_err("resolve ot phandles (ret=%d), %d\n", ret, onum); - goto out_free_np_overlay; - } - info->overlay_id = 0; ret = of_overlay_apply(info->np_overlay, &info->overlay_id); if (ret < 0) { pr_err("of_overlay_apply() (ret=%d), %d\n", ret, onum); + of_overlay_mutex_unlock(); goto out_free_np_overlay; } @@ -2209,7 +2216,10 @@ static __init void of_unittest_overlay_high_level(void) * Could not fixup phandles in unittest_unflatten_overlay_base() * because kmalloc() was not yet available. */ + of_overlay_mutex_lock(); of_resolve_phandles(overlay_base_root); + of_overlay_mutex_unlock(); + /* * do not allow overlay_base to duplicate any node already in diff --git a/include/linux/of.h b/include/linux/of.h index 49e5f24fb390..55e9ff0fb1fe 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -1273,9 +1273,6 @@ static inline int of_reconfig_get_state_change(unsigned long action, } #endif /* CONFIG_OF_DYNAMIC */ -/* CONFIG_OF_RESOLVE api */ -extern int of_resolve_phandles(struct device_node *tree); - /** * of_device_is_system_power_controller - Tells if system-power-controller is found for device_node * @np: Pointer to the given device_node