[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH for-4.12] dt: fix integer check in dt_device_is_compatible

Hi Stefano,

On 12/12/18 11:53 PM, Stefano Stabellini wrote:
On Wed, 12 Dec 2018, Julien Grall wrote:
Hi Stefano,

On 11/12/2018 18:46, Stefano Stabellini wrote:
cplen is unsigned, thus, it can never be < 0. Use a signed integer local
variable instead.

The current code checks > 0. Looking at the code I don't think it can ever be
negative. So can you details what is the problem you are trying to resolve?

Yes, it can be "negative" (not actually negative because it is defined
as unsigned), in fact it happens with the nodes added dynamically by grub
at boot. This patches fixes booting from grub.

Specifically ilen is initialed to 16, but strlen+1 is 17, so length
becomes -1. The length of the property generated by grub seems to be
short by 1.
Such explanation should have been in the commit message, it helps to diagnostics where the problem is coming from.

Looking at the specification [1] section 2.3.1, a compatible property is a concatenated list of null terminated strings. So the current code is actually correct and there is a bug in GRUB.

This bug was actually fixed by commit ae5817f1d "arm64/xen_boot: Fix Xen boot using GRUB2 on AARCH64".

I assume you are using Grub 2.02 which does not contain the full support for Xen. So I would not even consider to work-around it in Xen.

Instead, I would recommend you to upgrade to a more recent GRUB. It would be more likely staging as there are no new GRUB version.

Another solution is to use chainloading.


[1] https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.2

Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
   xen/common/device_tree.c | 7 +++++--
   1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 8fc401d..df274cc 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -213,17 +213,20 @@ bool_t dt_device_is_compatible(const struct
dt_device_node *device,
       const char* cp;
       u32 cplen, l;
+    s64 ilen;
         cp = dt_get_property(device, "compatible", &cplen);
       if ( cp == NULL )
           return 0;
-    while ( cplen > 0 )
+    ilen = cplen;
+    while ( ilen > 0 )
           if ( dt_compat_cmp(cp, compat) == 0 )
               return 1;
           l = strlen(cp) + 1;
           cp += l;
-        cplen -= l;
+        ilen -= l;
         return 0;

Julien Grall

Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.