From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v3 21/24] tools/(lib)xl: Add partial device tree support for ARM Date: Thu, 29 Jan 2015 12:02:22 +0000 Message-ID: <54CA214E.4010902@linaro.org> References: <1421159133-31526-1-git-send-email-julien.grall@linaro.org> <1421159133-31526-22-git-send-email-julien.grall@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YGnoC-0007l3-SQ for xen-devel@lists.xenproject.org; Thu, 29 Jan 2015 12:02:52 +0000 Received: by mail-wi0-f175.google.com with SMTP id fb4so25209688wid.2 for ; Thu, 29 Jan 2015 04:02:51 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: Wei Liu , ian.campbell@citrix.com, tim@xen.org, Ian Jackson , stefano.stabellini@citrix.com, xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On 29/01/15 11:03, Stefano Stabellini wrote: > On Tue, 13 Jan 2015, Julien Grall wrote: >> Let the user to pass additional nodes to the guest device tree. For this >> purpose, everything in the node /passthrough from the partial device tree will >> be copied into the guest device tree. >> >> The node /aliases will be also copied to allow the user to define aliases >> which can be used by the guest kernel. >> >> A simple partial device tree will look like: >> >> /dts-v1/; >> >> / { >> #address-cells = <2>; >> #size-cells = <2>; >> >> passthrough { >> compatible = "simple-bus"; >> ranges; >> #address-cells = <2>; >> #size-cells = <2>; >> >> /* List of your nodes */ >> } >> }; > > It would be nice to have an example of this under tools/examples. Ok. I will add one. [..] >> +/* >> + * Check if a string stored the strings block section is correctly >> + * nul-terminated. >> + * off_dt_strings and size_dt_strings fields have been validity-check >> + * earlier, so it's safe to use them here. >> + */ >> +static bool check_string(void *fdt, int nameoffset) >> +{ >> + const char *str = fdt_string(fdt, nameoffset); >> + >> + for (; nameoffset < fdt_size_dt_strings(fdt); nameoffset++, str++) { >> + if (*str == '\0') >> + return true; >> + } >> + >> + return false; >> +} > > strnlen? I could but it would not tell us directly if the string is NULL terminated or not. What about memchr? [..] >> +static int copy_node_by_path(libxl__gc *gc, const char *path, >> + void *fdt, void *pfdt) >> +{ >> + int nodeoff, r; >> + const char *name = strrchr(path, '/'); >> + >> + if (!name) >> + return -FDT_ERR_INTERNAL; >> + >> + name++; >> + >> + /* The FDT function to look at node doesn't take into account the >> + * unit (i.e anything after @) when search by name. Check if the >> + * name exactly match. >> + */ >> + nodeoff = fdt_path_offset(pfdt, path); >> + if (nodeoff < 0) >> + return nodeoff; >> + >> + if (strcmp(fdt_get_name(pfdt, nodeoff, NULL), name)) >> + return -FDT_ERR_NOTFOUND; > > Are we sure that the string returned by fdt_get_name is NULL terminated? Yes, libfdt does some sanity check on it (see fdt_next_tag case FDT_BEGIN_NODE). I tried to fix all the possible security flaw in libfdt (and there is quite a lot). If we don't trust the rest of libfdt, then we have to import our own and fix it. Regards, -- Julien Grall