All of lore.kernel.org
 help / color / mirror / Atom feed
* [bug report] pinctrl: add ZTE ZX pinctrl driver support
@ 2017-06-30  7:53 Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2017-06-30  7:53 UTC (permalink / raw)
  To: shawn.guo; +Cc: linux-gpio

Hello Shawn Guo,

This is a semi-automatic email about new static checker warnings.

The patch cbff0c4d27f4: "pinctrl: add ZTE ZX pinctrl driver support" 
from Apr 24, 2017, leads to the following Smatch complaint:

drivers/pinctrl/zte/pinctrl-zx.c:76 zx_set_mux()
	 warn: variable dereferenced before check 'data' (see line 67)

drivers/pinctrl/zte/pinctrl-zx.c
    66		struct zx_pin_data *data = pindesc->drv_data;
    67		struct zx_mux_desc *mux = data->muxes;
                                          ^^^^^^^^^^^
Dereference.

    68		u32 mask = (1 << data->width) - 1;
    69		u32 offset = data->offset;
    70		u32 bitpos = data->bitpos;
    71		struct function_desc *func;
    72		unsigned long flags;
    73		u32 val, mval;
    74	
    75		/* Skip reserved pin */
    76		if (!data)
                    ^^^^^
Too late.

    77			return -EINVAL;
    78	

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [bug report] pinctrl: add ZTE ZX pinctrl driver support
  2017-06-29 14:30 ` Shawn Guo
@ 2017-07-03 14:55   ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2017-07-03 14:55 UTC (permalink / raw)
  To: Shawn Guo; +Cc: linux-gpio

On Thu, Jun 29, 2017 at 10:30:55PM +0800, Shawn Guo wrote:
> Hi Dan,
> 
> On Thu, Jun 29, 2017 at 12:51:52PM +0300, Dan Carpenter wrote:
> > Hello Shawn Guo,
> > 
> > The patch cbff0c4d27f4: "pinctrl: add ZTE ZX pinctrl driver support"
> > from Apr 24, 2017, leads to the following static checker warning:
> > 
> > 	drivers/pinctrl/zte/pinctrl-zx.c:338 zx_pinctrl_build_state()
> > 	warn: passing devm_ allocated variable to kfree. 'functions'
> 
> I tried to see this warning with sparse, but failed.  Can you please
> tell how I can reproduced this static check warning?
> 

Hm.  It's a Smatch thing that I didn't push.  I'll push it in the next
day or two.

> > 
> > drivers/pinctrl/zte/pinctrl-zx.c
> >    293  
> >    294          /* Build function list from pin mux functions */
> >    295          functions = devm_kzalloc(&pdev->dev, info->npins * sizeof(*functions),
> >                 ^^^^^^^^^^^^^^^^^^^^^^^^
> > 
> >    296                                   GFP_KERNEL);
> >    297          if (!functions)
> >    298                  return -ENOMEM;
> >    299  
> >    300          nfunctions = 0;
> >    301          for (i = 0; i < info->npins; i++) {
> >    302                  const struct pinctrl_pin_desc *pindesc = info->pins + i;
> >    303                  struct zx_pin_data *data = pindesc->drv_data;
> >    304                  struct zx_mux_desc *mux;
> >    305  
> >    306                  /* Reserved pins do not have a drv_data at all */
> >    307                  if (!data)
> >    308                          continue;
> >    309  
> >    310                  /* Loop over all muxes for the pin */
> >    311                  mux = data->muxes;
> >    312                  while (mux->name) {
> >    313                          struct function_desc *func = functions;
> >    314  
> >    315                          /* Search function list for given mux */
> >    316                          while (func->name) {
> >    317                                  if (strcmp(mux->name, func->name) == 0) {
> >    318                                          /* Function exists */
> >    319                                          func->num_group_names++;
> >    320                                          break;
> >    321                                  }
> >    322                                  func++;
> >    323                          }
> >    324  
> >    325                          if (!func->name) {
> >    326                                  /* New function */
> >    327                                  func->name = mux->name;
> >    328                                  func->num_group_names = 1;
> >    329                                  radix_tree_insert(&pctldev->pin_function_tree,
> >    330                                                    nfunctions++, func);
> >    331                          }
> >    332  
> >    333                          mux++;
> >    334                  }
> >    335          }
> >    336  
> >    337          pctldev->num_functions = nfunctions;
> >    338          functions = krealloc(functions, nfunctions * sizeof(*functions),
> >                                      ^^^^^^^^^
> > Not allowed.
> 
> So I guess the best fix would be changing devm_kzalloc() above to
> kzalloc(), and managing the free procedure on my own, right?
>

Sounds good.

regards,
dan carpenter

> Shawn
> 
> > 
> >    339                               GFP_KERNEL);
> >    340  
> >    341          /* Find pin groups for every single function */
> >    342          for (i = 0; i < info->npins; i++) {

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [bug report] pinctrl: add ZTE ZX pinctrl driver support
  2017-06-29  9:51 Dan Carpenter
@ 2017-06-29 14:30 ` Shawn Guo
  2017-07-03 14:55   ` Dan Carpenter
  0 siblings, 1 reply; 4+ messages in thread
From: Shawn Guo @ 2017-06-29 14:30 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-gpio

Hi Dan,

On Thu, Jun 29, 2017 at 12:51:52PM +0300, Dan Carpenter wrote:
> Hello Shawn Guo,
> 
> The patch cbff0c4d27f4: "pinctrl: add ZTE ZX pinctrl driver support"
> from Apr 24, 2017, leads to the following static checker warning:
> 
> 	drivers/pinctrl/zte/pinctrl-zx.c:338 zx_pinctrl_build_state()
> 	warn: passing devm_ allocated variable to kfree. 'functions'

I tried to see this warning with sparse, but failed.  Can you please
tell how I can reproduced this static check warning?

> 
> drivers/pinctrl/zte/pinctrl-zx.c
>    293  
>    294          /* Build function list from pin mux functions */
>    295          functions = devm_kzalloc(&pdev->dev, info->npins * sizeof(*functions),
>                 ^^^^^^^^^^^^^^^^^^^^^^^^
> 
>    296                                   GFP_KERNEL);
>    297          if (!functions)
>    298                  return -ENOMEM;
>    299  
>    300          nfunctions = 0;
>    301          for (i = 0; i < info->npins; i++) {
>    302                  const struct pinctrl_pin_desc *pindesc = info->pins + i;
>    303                  struct zx_pin_data *data = pindesc->drv_data;
>    304                  struct zx_mux_desc *mux;
>    305  
>    306                  /* Reserved pins do not have a drv_data at all */
>    307                  if (!data)
>    308                          continue;
>    309  
>    310                  /* Loop over all muxes for the pin */
>    311                  mux = data->muxes;
>    312                  while (mux->name) {
>    313                          struct function_desc *func = functions;
>    314  
>    315                          /* Search function list for given mux */
>    316                          while (func->name) {
>    317                                  if (strcmp(mux->name, func->name) == 0) {
>    318                                          /* Function exists */
>    319                                          func->num_group_names++;
>    320                                          break;
>    321                                  }
>    322                                  func++;
>    323                          }
>    324  
>    325                          if (!func->name) {
>    326                                  /* New function */
>    327                                  func->name = mux->name;
>    328                                  func->num_group_names = 1;
>    329                                  radix_tree_insert(&pctldev->pin_function_tree,
>    330                                                    nfunctions++, func);
>    331                          }
>    332  
>    333                          mux++;
>    334                  }
>    335          }
>    336  
>    337          pctldev->num_functions = nfunctions;
>    338          functions = krealloc(functions, nfunctions * sizeof(*functions),
>                                      ^^^^^^^^^
> Not allowed.

So I guess the best fix would be changing devm_kzalloc() above to
kzalloc(), and managing the free procedure on my own, right?

Shawn

> 
>    339                               GFP_KERNEL);
>    340  
>    341          /* Find pin groups for every single function */
>    342          for (i = 0; i < info->npins; i++) {

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [bug report] pinctrl: add ZTE ZX pinctrl driver support
@ 2017-06-29  9:51 Dan Carpenter
  2017-06-29 14:30 ` Shawn Guo
  0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2017-06-29  9:51 UTC (permalink / raw)
  To: shawn.guo; +Cc: linux-gpio

Hello Shawn Guo,

The patch cbff0c4d27f4: "pinctrl: add ZTE ZX pinctrl driver support"
from Apr 24, 2017, leads to the following static checker warning:

	drivers/pinctrl/zte/pinctrl-zx.c:338 zx_pinctrl_build_state()
	warn: passing devm_ allocated variable to kfree. 'functions'

drivers/pinctrl/zte/pinctrl-zx.c
   293  
   294          /* Build function list from pin mux functions */
   295          functions = devm_kzalloc(&pdev->dev, info->npins * sizeof(*functions),
                ^^^^^^^^^^^^^^^^^^^^^^^^

   296                                   GFP_KERNEL);
   297          if (!functions)
   298                  return -ENOMEM;
   299  
   300          nfunctions = 0;
   301          for (i = 0; i < info->npins; i++) {
   302                  const struct pinctrl_pin_desc *pindesc = info->pins + i;
   303                  struct zx_pin_data *data = pindesc->drv_data;
   304                  struct zx_mux_desc *mux;
   305  
   306                  /* Reserved pins do not have a drv_data at all */
   307                  if (!data)
   308                          continue;
   309  
   310                  /* Loop over all muxes for the pin */
   311                  mux = data->muxes;
   312                  while (mux->name) {
   313                          struct function_desc *func = functions;
   314  
   315                          /* Search function list for given mux */
   316                          while (func->name) {
   317                                  if (strcmp(mux->name, func->name) == 0) {
   318                                          /* Function exists */
   319                                          func->num_group_names++;
   320                                          break;
   321                                  }
   322                                  func++;
   323                          }
   324  
   325                          if (!func->name) {
   326                                  /* New function */
   327                                  func->name = mux->name;
   328                                  func->num_group_names = 1;
   329                                  radix_tree_insert(&pctldev->pin_function_tree,
   330                                                    nfunctions++, func);
   331                          }
   332  
   333                          mux++;
   334                  }
   335          }
   336  
   337          pctldev->num_functions = nfunctions;
   338          functions = krealloc(functions, nfunctions * sizeof(*functions),
                                     ^^^^^^^^^
Not allowed.

   339                               GFP_KERNEL);
   340  
   341          /* Find pin groups for every single function */
   342          for (i = 0; i < info->npins; i++) {

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-07-03 14:55 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-30  7:53 [bug report] pinctrl: add ZTE ZX pinctrl driver support Dan Carpenter
  -- strict thread matches above, loose matches on Subject: below --
2017-06-29  9:51 Dan Carpenter
2017-06-29 14:30 ` Shawn Guo
2017-07-03 14:55   ` Dan Carpenter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.