From patchwork Tue Feb 14 02:50:40 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Boyd X-Patchwork-Id: 9571069 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 03EBD60578 for ; Tue, 14 Feb 2017 02:51:18 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E4BD0267EC for ; Tue, 14 Feb 2017 02:51:17 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D8CBE27FAE; Tue, 14 Feb 2017 02:51:17 +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=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 763EC267EC for ; Tue, 14 Feb 2017 02:51:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:Message-Id:Date: Subject: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-Owner; bh=7q13A0B9o9lv7JQ7be/VhbolXsHoYxRRwn8QirLkD9U=; b=CWn yxNsrO2tzP/t9rPoACSPzugSagUahQL+/L7emJR0lI2nWHoVFAmvEkqtAu7twjcdJAFVvbikG+VTX HhY3lKnzXFjSE8eq2L5/728dbfB4y/BXcE+ChrNTuTw5aQNEpZWYF1tA+PxiLgfVkVCcGxQpqfR2R W0gszqQxvnbUYFQHJ46q4wF02ZtwpFQXKcROkAQIzMKqjsyK8vW1aKz5W4iQAbuT9gX9SRRoqYcO0 YxmtoXNVbiZttPeMzIaQBoQ7JpTk0omEZh/fFngP1NJzGGYpVLytavsLPMrEfgesLpOlWI/X5QoQW 99aFkEk82kB+bzppklSjRGFIc0DUkgw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1cdTCs-0003vZ-Vp; Tue, 14 Feb 2017 02:51:06 +0000 Received: from mail-it0-x235.google.com ([2607:f8b0:4001:c0b::235]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1cdTCp-0003uN-K9 for linux-arm-kernel@lists.infradead.org; Tue, 14 Feb 2017 02:51:05 +0000 Received: by mail-it0-x235.google.com with SMTP id c7so14967393itd.1 for ; Mon, 13 Feb 2017 18:50:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=8YEYE9GuWehgGHTCsiHy8OjujOmJCm5b3ZKOuIIdZWM=; b=WRIz+kcQghFEUor2nZA/VlRbEuS1k8Z5wUgP1llNTJe8psbXsT4PYgNN24M32Nmwpj fhU6B2qN7oiJ+zOO56PnMHczTG0FtlogIb55Qad6r1B4N6qqlIdQS7wJqsgTPYm8/vva dOZnboB/Ga50fgHE2L2zbPBDA9ndO6dZwj7sU= 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; bh=8YEYE9GuWehgGHTCsiHy8OjujOmJCm5b3ZKOuIIdZWM=; b=HaiwphBkXDsJbL8uq9bu/WwNjPAhaebBnJ84fTjmUmxL87ppeaNlrJ/TceZhDHnbQz LAcjtSZTQnMM7Qqrq0cRkYL+m0Px8lLipY5kx/kMtBOHhdc+IP/r6JhdVa5eGAQyjOOQ teUeCgOUtWBCc2Rh6R+k6YB7c0D9QT4P7nKLwksdg18EDdkOxmKwY1sr5F7lc8ZdSmOo TjThvOGqSyi5QILrm/s3xXVV+CuWcfKoEu658/TJkYwwY6OWsW4ekQQwD6bVnMeBcVDT 5CJvtt9BeGSbIYVv4kxrfmhqtPqz29bQu/YPn7tZJgMK9Y57L9yGHaVHnjnJZQ5DSixG d9RQ== X-Gm-Message-State: AMke39k/y7mpaV5J5OjhBuAjaCOvQE70FFnB88ZcFW8Zx/F2F2X8sFQH2gVNHJISafLpg0HI X-Received: by 10.84.141.164 with SMTP id 33mr33613038plv.86.1487040642242; Mon, 13 Feb 2017 18:50:42 -0800 (PST) Received: from localhost.localdomain (i-global254.qualcomm.com. [199.106.103.254]) by smtp.gmail.com with ESMTPSA id r78sm23264183pfe.55.2017.02.13.18.50.40 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 13 Feb 2017 18:50:41 -0800 (PST) From: Stephen Boyd To: Rob Herring , Frank Rowand Subject: [RFC/PATCH] of: Mark property::value as const Date: Mon, 13 Feb 2017 18:50:40 -0800 Message-Id: <20170214025040.23955-1-stephen.boyd@linaro.org> X-Mailer: git-send-email 2.10.0.297.gf6727b0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170213_185103_715707_1F85484E X-CRM114-Status: GOOD ( 19.50 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP The 'blob' we pass into populate_properties() is marked as const, but we cast that const away when we assign the result of fdt_getprop_by_offset() to pp->value. Let's mark value as const instead, so that code can't mistakenly write to the value of the property that we've so far advertised as const. Unfortunately, this exposes a problem with the fdt resolver code, where we overwrite the value member of properties of phandles to update them with their final value. Add a comment for now to indicate where we're potentially writing over const data. You can see the problem here by loading an overlay dtb into the kernel via the request firmware helper method (not direct loading) and then passing that tree to the resolver on an arm64 device. In this case, the firmware data is vmapped with KERNEL_PAGE_RO and the code crashes when attempting to write to the blob to update the phandle properties. Signed-off-by: Stephen Boyd --- I was thinking perhaps it would work to store another __be32 variant of the phandle in each device node, but then we still have a problem with properties that have phandles inside them at some offset that we need to update. I guess the only real solution is to deep copy the property in that case and then save around some info to free the duplicated property later on? drivers/of/base.c | 2 +- drivers/of/fdt.c | 12 ++++++------ drivers/of/resolver.c | 3 +++ include/linux/of.h | 2 +- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index fb6bb855714e..8e5f29b814c9 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1156,7 +1156,7 @@ EXPORT_SYMBOL_GPL(of_property_count_elems_of_size); * property data is too small or too large. * */ -static void *of_find_property_value_of_size(const struct device_node *np, +static const void *of_find_property_value_of_size(const struct device_node *np, const char *propname, u32 min, u32 max, size_t *len) { struct property *prop = of_find_property(np, propname, NULL); diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index c9b5cac03b36..0635ef5dabf3 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -222,7 +222,7 @@ static void populate_properties(const void *blob, pp->name = (char *)pname; pp->length = sz; - pp->value = (__be32 *)val; + pp->value = val; *pprev = pp; pprev = &pp->next; } @@ -232,6 +232,7 @@ static void populate_properties(const void *blob, */ if (!has_name) { const char *p = nodename, *ps = p, *pa = NULL; + char *b; int len; while (*p) { @@ -250,13 +251,12 @@ static void populate_properties(const void *blob, if (!dryrun) { pp->name = "name"; pp->length = len; - pp->value = pp + 1; + pp->value = b = (char *)(pp + 1); *pprev = pp; pprev = &pp->next; - memcpy(pp->value, ps, len - 1); - ((char *)pp->value)[len - 1] = 0; - pr_debug("fixed up name for %s -> %s\n", - nodename, (char *)pp->value); + memcpy(b, ps, len - 1); + b[len - 1] = 0; + pr_debug("fixed up name for %s -> %s\n", nodename, b); } } diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c index 8bf12e904fd2..6d88f8100318 100644 --- a/drivers/of/resolver.c +++ b/drivers/of/resolver.c @@ -93,6 +93,7 @@ static void adjust_overlay_phandles(struct device_node *overlay, if (phandle == OF_PHANDLE_ILLEGAL) continue; + /* This is bad because we cast away const */ *(uint32_t *)prop->value = cpu_to_be32(overlay->phandle); } @@ -154,6 +155,7 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay, goto err_fail; } + /* This is bad because we cast away const */ *(__be32 *)(prop->value + offset) = cpu_to_be32(phandle); } @@ -222,6 +224,7 @@ static int adjust_local_phandle_references(struct device_node *local_fixups, phandle = be32_to_cpu(*(__be32 *)(prop->value + off)); phandle += phandle_delta; + /* This is bad because we cast away const */ *(__be32 *)(prop->value + off) = cpu_to_be32(phandle); } } diff --git a/include/linux/of.h b/include/linux/of.h index f22d4a83ca07..b0253886ef46 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -35,7 +35,7 @@ typedef u32 ihandle; struct property { char *name; int length; - void *value; + const void *value; struct property *next; unsigned long _flags; unsigned int unique_id;