Re: [coreboot] NULL pointer dereference in search_bus_device()

[Sven Schnelle]

"Myles Watson"  writes:
> > diff --git a/src/devices/device_util.c b/src/devices/device_util.c
> > index 9081a36..d761cba 100644
> > --- a/src/devices/device_util.c
> > +++ b/src/devices/device_util.c
> > @@ -583,8 +583,9 @@ void search_bus_resources(struct bus *bus, unsigned
> > long type_mask,
> >    				if (subbus->link_num
> >    				==
>
> IOINDEX_SUBTRACTIVE_LINK(res->index))
> > 				break;
> > 		search_bus_resources(subbus, type_mask,
>
> type,
> > 				     search, gp);
> > 		if (subbus)
> > 			search_bus_resources(subbus,
>
> type_mask,
> > type,
> > 						search, gp);
> > 	continue;
> > }
> > search(gp, curdev, res);
>
> If subbus is NULL, then accessing subbus->link_num is also a problem.

That doesn't happen, because the if (subbus... is in the for loop, which
checks for NULL. the search_bus_resources() is always called outside the
for loop.

current code:
-----------------------8<-------------------
if (res->flags & IORESOURCE_SUBTRACTIVE) {
    struct bus * subbus;
    for (subbus = curdev->link_list; subbus; subbus = subbus->next)
        if (subbus->link_num == IOINDEX_SUBTRACTIVE_LINK(res->index))
                   break;
       search_bus_resources(subbus, type_mask, type, search, gp);
       continue;
    }   
-----------------------8<-------------------

it should be proably something like:
-----------------------8<-------------------

if (res->flags & IORESOURCE_SUBTRACTIVE) {
    struct bus * subbus;
    for (subbus = curdev->link_list; subbus; subbus = subbus->next) {
                if (subbus->link_num == IOINDEX_SUBTRACTIVE_LINK(res->index))
                    search_bus_resources(subbus, type_mask, type, search, gp);
                 break;
    }
       continue;
}   
-----------------------8<-------------------

Regards,

Sven.