All of lore.kernel.org
 help / color / mirror / Atom feed
* [bug report] extcon: add Realtek DHC RTD SoC Type-C driver
@ 2023-10-10  9:27 Dan Carpenter
  2023-10-16  5:48 ` Stanley Chang[昌育德]
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2023-10-10  9:27 UTC (permalink / raw)
  To: stanley_chang; +Cc: kernel-janitors

Hello Stanley Chang,

The patch 8a590d7371f0: "extcon: add Realtek DHC RTD SoC Type-C
driver" from Sep 4, 2023 (linux-next), leads to the following Smatch
static checker warning:

	drivers/extcon/extcon-rtk-type-c.c:905 __updated_type_c_parameter_by_efuse()
	error: 'buf' dereferencing possible ERR_PTR()

drivers/extcon/extcon-rtk-type-c.c
    873 static int __updated_type_c_parameter_by_efuse(struct type_c_data *type_c)
    874 {
    875         struct type_c_cfg *type_c_cfg = type_c->type_c_cfg;
    876         struct cc_param *cc_param;
    877         struct nvmem_cell *cell;
    878         s8 cc1_4p7k = 0;
    879         s8 cc1_12k = 0;
    880         s8 cc1_0p2v = 0;
    881         s8 cc1_0p8v = 0;
    882         s8 cc1_2p6v = 0;
    883         s8 cc1_0p66v = 0;
    884         s8 cc1_1p23v = 0;
    885         s8 cc2_4p7k = 0;
    886         s8 cc2_12k = 0;
    887         s8 cc2_0p2v = 0;
    888         s8 cc2_0p8v = 0;
    889         s8 cc2_2p6v = 0;
    890         s8 cc2_0p66v = 0;
    891         s8 cc2_1p23v = 0;
    892 
    893         cell = nvmem_cell_get(type_c->dev, "usb-cal");
    894         if (IS_ERR(cell)) {
    895                 dev_warn(type_c->dev, "%s failed to get usb-cal: %ld\n",
    896                          __func__, PTR_ERR(cell));
    897         } else {
    898                 unsigned char *buf;
    899                 size_t buf_size;
    900                 int value_size = 4;
    901                 int value_mask = (BIT(value_size) - 1);
    902 
    903                 buf = nvmem_cell_read(cell, &buf_size);
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This does a variable size allocation.  Not sure how large.  But
allocations need to be checked for failure.

    904 
--> 905                 cc1_0p2v = get_value((buf[0] >> value_size * 0) & value_mask);
                                              ^^^^^^
Or it leads to a crash.

    906                 cc1_0p8v = get_value((buf[0] >> value_size * 1) & value_mask);
    907                 cc1_2p6v = get_value((buf[1] >> value_size * 0) & value_mask);
    908                 cc1_0p66v = get_value((buf[1] >> value_size * 1) & value_mask);
    909                 cc1_1p23v = get_value((buf[2] >> value_size * 0) & value_mask);
    910 
    911                 cc2_0p2v = get_value((buf[3] >> value_size * 0) & value_mask);
    912                 cc2_0p8v = get_value((buf[3] >> value_size * 1) & value_mask);
    913                 cc2_2p6v = get_value((buf[4] >> value_size * 0) & value_mask);
    914                 cc2_0p66v = get_value((buf[4] >> value_size * 1) & value_mask);
    915                 cc2_1p23v = get_value((buf[5] >> value_size * 0) & value_mask);
    916 
    917                 cc1_4p7k = get_value((buf[6] >> value_size * 0) & value_mask);
    918                 cc1_12k = get_value((buf[6] >> value_size * 1) & value_mask);
    919                 cc2_4p7k = get_value((buf[7] >> value_size * 0) & value_mask);
    920                 cc2_12k = get_value((buf[7] >> value_size * 1) & value_mask);
    921 
    922                 kfree(buf);
    923                 nvmem_cell_put(cell);
    924         }
    925 
    926         dev_dbg(type_c->dev, "check efuse cc1_4p7k=%d cc1_12k=%d cc2_4p7k=%d cc2_12k=%d\n",
    927                 cc1_4p7k, cc1_12k, cc2_4p7k, cc2_12k);
    928         dev_dbg(type_c->dev, "check efuse cc1_0p2v=%d cc1_0p8v=%d cc1_2p6v=%d cc1_0p66v=%d cc1_1p23v=%d\n",
    929                 cc1_0p2v, cc1_0p8v, cc1_2p6v, cc1_0p66v, cc1_1p23v);
    930         dev_dbg(type_c->dev, "check efuse cc2_0p2v=%d cc2_0p8v=%d cc2_2p6v=%d cc2_0p66v=%d cc2_1p23v=%d\n",
    931                 cc2_0p2v, cc2_0p8v, cc2_2p6v, cc2_0p66v, cc2_1p23v);
    932 
    933         cc_param = &type_c_cfg->cc1_param;
    934         cc_param->rp_4p7k_code = cc_param->rp_4p7k_code + cc1_4p7k;
    935         cc_param->rp_12k_code = cc_param->rp_12k_code + cc1_12k;
    936 
    937         cc_param->vref_1p23v = cc_param->vref_1p23v + cc1_1p23v;
    938         cc_param->vref_0p66v = cc_param->vref_0p66v + cc1_0p66v;
    939         cc_param->vref_2p6v = cc_param->vref_2p6v + cc1_2p6v;
    940         cc_param->vref_0p8v = cc_param->vref_0p8v + cc1_0p8v;
    941         cc_param->vref_0p2v = cc_param->vref_0p2v + cc1_0p2v;
    942 
    943         cc_param = &type_c_cfg->cc2_param;
    944         cc_param->rp_4p7k_code = cc_param->rp_4p7k_code + cc2_4p7k;
    945         cc_param->rp_12k_code = cc_param->rp_12k_code + cc2_12k;
    946 
    947         cc_param->vref_1p23v = cc_param->vref_1p23v + cc2_1p23v;
    948         cc_param->vref_0p66v = cc_param->vref_0p66v + cc2_0p66v;
    949         cc_param->vref_2p6v = cc_param->vref_2p6v + cc2_2p6v;
    950         cc_param->vref_0p8v = cc_param->vref_0p8v + cc2_0p8v;
    951         cc_param->vref_0p2v = cc_param->vref_0p2v + cc2_0p2v;
    952 
    953         return 0;
    954 }

regards,
dan carpenter

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

* RE: [bug report] extcon: add Realtek DHC RTD SoC Type-C driver
  2023-10-10  9:27 [bug report] extcon: add Realtek DHC RTD SoC Type-C driver Dan Carpenter
@ 2023-10-16  5:48 ` Stanley Chang[昌育德]
  0 siblings, 0 replies; 2+ messages in thread
From: Stanley Chang[昌育德] @ 2023-10-16  5:48 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: kernel-janitors@vger.kernel.org

Hi Dan,

I have fixed this issue.
https://lore.kernel.org/lkml/20231016053510.28881-1-stanley_chang@realtek.com/

Thanks,
Stanley

> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@linaro.org>
> Sent: Tuesday, October 10, 2023 5:27 PM
> To: Stanley Chang[昌育德] <stanley_chang@realtek.com>
> Cc: kernel-janitors@vger.kernel.org
> Subject: [bug report] extcon: add Realtek DHC RTD SoC Type-C driver
> 
> 
> External mail.
> 
> 
> 
> Hello Stanley Chang,
> 
> The patch 8a590d7371f0: "extcon: add Realtek DHC RTD SoC Type-C driver"
> from Sep 4, 2023 (linux-next), leads to the following Smatch static checker
> warning:
> 
>         drivers/extcon/extcon-rtk-type-c.c:905
> __updated_type_c_parameter_by_efuse()
>         error: 'buf' dereferencing possible ERR_PTR()
> 
> drivers/extcon/extcon-rtk-type-c.c
>     873 static int __updated_type_c_parameter_by_efuse(struct type_c_data
> *type_c)
>     874 {
>     875         struct type_c_cfg *type_c_cfg = type_c->type_c_cfg;
>     876         struct cc_param *cc_param;
>     877         struct nvmem_cell *cell;
>     878         s8 cc1_4p7k = 0;
>     879         s8 cc1_12k = 0;
>     880         s8 cc1_0p2v = 0;
>     881         s8 cc1_0p8v = 0;
>     882         s8 cc1_2p6v = 0;
>     883         s8 cc1_0p66v = 0;
>     884         s8 cc1_1p23v = 0;
>     885         s8 cc2_4p7k = 0;
>     886         s8 cc2_12k = 0;
>     887         s8 cc2_0p2v = 0;
>     888         s8 cc2_0p8v = 0;
>     889         s8 cc2_2p6v = 0;
>     890         s8 cc2_0p66v = 0;
>     891         s8 cc2_1p23v = 0;
>     892
>     893         cell = nvmem_cell_get(type_c->dev, "usb-cal");
>     894         if (IS_ERR(cell)) {
>     895                 dev_warn(type_c->dev, "%s failed to get usb-cal:
> %ld\n",
>     896                          __func__, PTR_ERR(cell));
>     897         } else {
>     898                 unsigned char *buf;
>     899                 size_t buf_size;
>     900                 int value_size = 4;
>     901                 int value_mask = (BIT(value_size) - 1);
>     902
>     903                 buf = nvmem_cell_read(cell, &buf_size);
>                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> This does a variable size allocation.  Not sure how large.  But allocations
> need to be checked for failure.
> 
>     904
> --> 905                 cc1_0p2v = get_value((buf[0] >> value_size * 0) &
> value_mask);
>                                               ^^^^^^ Or it leads to
> a crash.
> 
>     906                 cc1_0p8v = get_value((buf[0] >> value_size * 1)
> & value_mask);
>     907                 cc1_2p6v = get_value((buf[1] >> value_size * 0)
> & value_mask);
>     908                 cc1_0p66v = get_value((buf[1] >> value_size * 1)
> & value_mask);
>     909                 cc1_1p23v = get_value((buf[2] >> value_size * 0)
> & value_mask);
>     910
>     911                 cc2_0p2v = get_value((buf[3] >> value_size * 0)
> & value_mask);
>     912                 cc2_0p8v = get_value((buf[3] >> value_size * 1)
> & value_mask);
>     913                 cc2_2p6v = get_value((buf[4] >> value_size * 0)
> & value_mask);
>     914                 cc2_0p66v = get_value((buf[4] >> value_size * 1)
> & value_mask);
>     915                 cc2_1p23v = get_value((buf[5] >> value_size * 0)
> & value_mask);
>     916
>     917                 cc1_4p7k = get_value((buf[6] >> value_size * 0)
> & value_mask);
>     918                 cc1_12k = get_value((buf[6] >> value_size * 1) &
> value_mask);
>     919                 cc2_4p7k = get_value((buf[7] >> value_size * 0)
> & value_mask);
>     920                 cc2_12k = get_value((buf[7] >> value_size * 1) &
> value_mask);
>     921
>     922                 kfree(buf);
>     923                 nvmem_cell_put(cell);
>     924         }
>     925
>     926         dev_dbg(type_c->dev, "check efuse cc1_4p7k=%d
> cc1_12k=%d cc2_4p7k=%d cc2_12k=%d\n",
>     927                 cc1_4p7k, cc1_12k, cc2_4p7k, cc2_12k);
>     928         dev_dbg(type_c->dev, "check efuse cc1_0p2v=%d
> cc1_0p8v=%d cc1_2p6v=%d cc1_0p66v=%d cc1_1p23v=%d\n",
>     929                 cc1_0p2v, cc1_0p8v, cc1_2p6v, cc1_0p66v,
> cc1_1p23v);
>     930         dev_dbg(type_c->dev, "check efuse cc2_0p2v=%d
> cc2_0p8v=%d cc2_2p6v=%d cc2_0p66v=%d cc2_1p23v=%d\n",
>     931                 cc2_0p2v, cc2_0p8v, cc2_2p6v, cc2_0p66v,
> cc2_1p23v);
>     932
>     933         cc_param = &type_c_cfg->cc1_param;
>     934         cc_param->rp_4p7k_code = cc_param->rp_4p7k_code +
> cc1_4p7k;
>     935         cc_param->rp_12k_code = cc_param->rp_12k_code +
> cc1_12k;
>     936
>     937         cc_param->vref_1p23v = cc_param->vref_1p23v +
> cc1_1p23v;
>     938         cc_param->vref_0p66v = cc_param->vref_0p66v +
> cc1_0p66v;
>     939         cc_param->vref_2p6v = cc_param->vref_2p6v + cc1_2p6v;
>     940         cc_param->vref_0p8v = cc_param->vref_0p8v + cc1_0p8v;
>     941         cc_param->vref_0p2v = cc_param->vref_0p2v + cc1_0p2v;
>     942
>     943         cc_param = &type_c_cfg->cc2_param;
>     944         cc_param->rp_4p7k_code = cc_param->rp_4p7k_code +
> cc2_4p7k;
>     945         cc_param->rp_12k_code = cc_param->rp_12k_code +
> cc2_12k;
>     946
>     947         cc_param->vref_1p23v = cc_param->vref_1p23v +
> cc2_1p23v;
>     948         cc_param->vref_0p66v = cc_param->vref_0p66v +
> cc2_0p66v;
>     949         cc_param->vref_2p6v = cc_param->vref_2p6v + cc2_2p6v;
>     950         cc_param->vref_0p8v = cc_param->vref_0p8v + cc2_0p8v;
>     951         cc_param->vref_0p2v = cc_param->vref_0p2v + cc2_0p2v;
>     952
>     953         return 0;
>     954 }
> 
> regards,
> dan carpenter

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

end of thread, other threads:[~2023-10-16  5:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-10  9:27 [bug report] extcon: add Realtek DHC RTD SoC Type-C driver Dan Carpenter
2023-10-16  5:48 ` Stanley Chang[昌育德]

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.