Skip to contentSkip to conversation

of: base: Introduce of_alias_get_alias_list() to check alias IDs

bug report Dan Carpenter 3 participants 3 messages Mar 15, 2022 – Mar 21, 2022

lore
Share & tools
lore
#0 lore raw

Hello Michal Simek,

The patch b1078c355d76: "of: base: Introduce of_alias_get_alias_list() to check alias IDs" from Sep 20, 2018, leads to the following Smatch static checker warning:

	drivers/of/base.c:2038 of_alias_get_alias_list()
	warn: passing negative bit value 's32min-(-2),0-s32max' to 'set_bit()'

drivers/of/base.c

    2028                 }
    2029 
    2030                 if (of_match_node(matches, app->np)) {
    2031                         pr_debug("%s: Allocated ID %d\n", __func__, app->id);
    2032 
    2033                         if (app->id >= nbits) {
    2034                                 pr_warn("%s: ID %d >= than bitmap field %d\n",
    2035                                         __func__, app->id, nbits);
    2036                                 ret = -EOVERFLOW;
    2037                         } else {

--> 2038 set_bit(app->id, bitmap);

Yes, there is a possibility for passing negative bits to set_bit(). But don't these bounds checks really belong in of_alias_add()? Adding invalid aliases seems like a risky thing.

    2039                         }
    2040                 }
    2041         }
    2042         mutex_unlock(&of_mutex);
    2043 
    2044         return ret;
    2045 }

regards,
dan carpenter

#1 lore raw
Quoted 9 lines · Dan Carpenter

Hello Michal Simek,

The patch b1078c355d76: "of: base: Introduce of_alias_get_alias_list() to check alias IDs" from Sep 20, 2018, leads to the following Smatch static checker warning:

        drivers/of/base.c:2038 of_alias_get_alias_list()
        warn: passing negative bit value 's32min-(-2),0-s32max' to 'set_bit()'

The 1 user of of_alias_get_alias_list() has since been refactored and doesn't use of_alias_get_alias_list() anymore. So I'm just going to revert b1078c355d76.

Rob

#2 lore raw
Quoted 8 lines · Rob Herring
Quoted 7 lines · Dan Carpenter

Hello Michal Simek,

The patch b1078c355d76: "of: base: Introduce of_alias_get_alias_list() to check alias IDs" from Sep 20, 2018, leads to the following Smatch static checker warning:

Quoted 6 lines
Quoted 2 lines
         drivers/of/base.c:2038 of_alias_get_alias_list()
         warn: passing negative bit value 's32min-(-2),0-s32max' to 'set_bit()'

The 1 user of of_alias_get_alias_list() has since been refactored and doesn't use of_alias_get_alias_list() anymore. So I'm just going to revert b1078c355d76.

yes. I am fine with it. We can get it back and fix it when it is again needed.

Thanks,
Michal