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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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.


> > 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
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel