Re: [bug report] pinctrl: Add driver for Sunplus SP7021

[呂芳騰 <wellslutw@gmail.com>]

Hi Dan Carpenter,

Thanks for inform me the issue. I'll send to patch to fix this issue.

Best Regards,
Wells Lu

Dan Carpenter <dan.carpenter@linaro.org> 於 2023年5月22日 週一 下午3:49寫道：
>
> Hello Wells Lu,
>
> The patch aa74c44be19c: "pinctrl: Add driver for Sunplus SP7021" from
> Jan 16, 2022, leads to the following Smatch static checker warning:
>
> drivers/pinctrl/sunplus/sppctl.c:886 sppctl_dt_node_to_map() error: potential null dereference 'configs'.  (kmalloc returns null)
> drivers/pinctrl/sunplus/sppctl.c:899 sppctl_dt_node_to_map() error: potential null dereference 'configs'.  (kmalloc returns null)
>
> drivers/pinctrl/sunplus/sppctl.c
>     820 static int sppctl_dt_node_to_map(struct pinctrl_dev *pctldev, struct device_node *np_config,
>     821                                  struct pinctrl_map **map, unsigned int *num_maps)
>     822 {
>     823         struct sppctl_pdata *pctl = pinctrl_dev_get_drvdata(pctldev);
>     824         int nmG = of_property_count_strings(np_config, "groups");
>     825         const struct sppctl_func *f = NULL;
>     826         u8 pin_num, pin_type, pin_func;
>     827         struct device_node *parent;
>     828         unsigned long *configs;
>     829         struct property *prop;
>     830         const char *s_f, *s_g;
>     831
>     832         const __be32 *list;
>     833         u32 dt_pin, dt_fun;
>     834         int i, size = 0;
>     835
>     836         list = of_get_property(np_config, "sunplus,pins", &size);
>     837
>     838         if (nmG <= 0)
>     839                 nmG = 0;
>     840
>     841         parent = of_get_parent(np_config);
>     842         *num_maps = size / sizeof(*list);
>     843
>     844         /*
>     845          * Process property:
>     846          *     sunplus,pins = < u32 u32 u32 ... >;
>     847          *
>     848          * Each 32-bit integer defines a individual pin in which:
>     849          *
>     850          *   Bit 32~24: defines GPIO pin number. Its range is 0 ~ 98.
>     851          *   Bit 23~16: defines types: (1) fully-pinmux pins
>     852          *                             (2) IO processor pins
>     853          *                             (3) digital GPIO pins
>     854          *   Bit 15~8:  defines pins of peripherals (which are defined in
>     855          *              'include/dt-binging/pinctrl/sppctl.h').
>     856          *   Bit 7~0:   defines types or initial-state of digital GPIO pins.
>     857          */
>     858         for (i = 0; i < (*num_maps); i++) {
>     859                 dt_pin = be32_to_cpu(list[i]);
>     860                 pin_num = FIELD_GET(GENMASK(31, 24), dt_pin);
>     861
>     862                 if (pin_num >= sppctl_pins_all_sz) {
>     863                         dev_err(pctldev->dev, "Invalid pin property at index %d (0x%08x)\n",
>     864                                 i, dt_pin);
>     865                         return -EINVAL;
>     866                 }
>     867         }
>     868
>     869         *map = kcalloc(*num_maps + nmG, sizeof(**map), GFP_KERNEL);
>     870         if (*map == NULL)
>     871                 return -ENOMEM;
>     872
>     873         for (i = 0; i < (*num_maps); i++) {
>     874                 dt_pin = be32_to_cpu(list[i]);
>     875                 pin_num = FIELD_GET(GENMASK(31, 24), dt_pin);
>     876                 pin_type = FIELD_GET(GENMASK(23, 16), dt_pin);
>     877                 pin_func = FIELD_GET(GENMASK(15, 8), dt_pin);
>     878                 (*map)[i].name = parent->name;
>     879
>     880                 if (pin_type == SPPCTL_PCTL_G_GPIO) {
>     881                         /* A digital GPIO pin */
>     882                         (*map)[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
>     883                         (*map)[i].data.configs.num_configs = 1;
>     884                         (*map)[i].data.configs.group_or_pin = pin_get_name(pctldev, pin_num);
>     885                         configs = kmalloc(sizeof(*configs), GFP_KERNEL);
>                                 ^^^^^^^^^^^^^^^^^^
> Static checkers like for kmalloc to be checked.
>
> --> 886                         *configs = FIELD_GET(GENMASK(7, 0), dt_pin);
>     887                         (*map)[i].data.configs.configs = configs;
>     888
>     889                         dev_dbg(pctldev->dev, "%s: GPIO (%s)\n",
>     890                                 (*map)[i].data.configs.group_or_pin,
>     891                                 (*configs & (SPPCTL_PCTL_L_OUT | SPPCTL_PCTL_L_OU1)) ?
>     892                                 "OUT" : "IN");
>     893                 } else if (pin_type == SPPCTL_PCTL_G_IOPP) {
>     894                         /* A IO Processor (IOP) pin */
>     895                         (*map)[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
>     896                         (*map)[i].data.configs.num_configs = 1;
>     897                         (*map)[i].data.configs.group_or_pin = pin_get_name(pctldev, pin_num);
>     898                         configs = kmalloc(sizeof(*configs), GFP_KERNEL);
>                                 ^^^^^^^^^^^^^^^^^^
> Here too.
>
>     899                         *configs = SPPCTL_IOP_CONFIGS;
>     900                         (*map)[i].data.configs.configs = configs;
>     901
>     902                         dev_dbg(pctldev->dev, "%s: IOP\n",
>     903                                 (*map)[i].data.configs.group_or_pin);
>     904                 } else {
>     905                         /* A fully-pinmux pin */
>
> regards,
> dan carpenter