[bug report] ASoC: mediatek: mt8188: support etdm in platform driver

Dan Carpenter error27 at gmail.com
Wed Jan 25 04:13:58 PST 2023


Hello Trevor Wu,

The patch 2babb4777489: "ASoC: mediatek: mt8188: support etdm in
platform driver" from Jan 16, 2023, leads to the following Smatch
static checker warning:

	sound/soc/mediatek/mt8188/mt8188-dai-etdm.c:2487 mt8188_dai_etdm_parse_of()
	warn: 'ret' returned from snprintf() might be larger than 48

sound/soc/mediatek/mt8188/mt8188-dai-etdm.c
    2455 static void mt8188_dai_etdm_parse_of(struct mtk_base_afe *afe)
    2456 {
    2457         const struct device_node *of_node = afe->dev->of_node;
    2458         struct mt8188_afe_private *afe_priv = afe->platform_priv;
    2459         struct mtk_dai_etdm_priv *etdm_data;
    2460         char prop[48];
    2461         u8 disable_chn[MT8188_ETDM_MAX_CHANNELS];
    2462         int max_chn = MT8188_ETDM_MAX_CHANNELS;
    2463         unsigned int sync_id;
    2464         u32 sel;
    2465         int ret;
    2466         int dai_id;
    2467         int i, j;
    2468         struct {
    2469                 const char *name;
    2470                 const unsigned int sync_id;
    2471         } of_afe_etdms[MT8188_AFE_IO_ETDM_NUM] = {
    2472                 {"etdm-in1", ETDM_SYNC_FROM_IN1},
    2473                 {"etdm-in2", ETDM_SYNC_FROM_IN2},
    2474                 {"etdm-out1", ETDM_SYNC_FROM_OUT1},
    2475                 {"etdm-out2", ETDM_SYNC_FROM_OUT2},
    2476                 {"etdm-out3", ETDM_SYNC_FROM_OUT3},
    2477         };
    2478 
    2479         for (i = 0; i < MT8188_AFE_IO_ETDM_NUM; i++) {
    2480                 dai_id = ETDM_TO_DAI_ID(i);
    2481                 etdm_data = afe_priv->dai_priv[dai_id];
    2482 
    2483                 ret = snprintf(prop, sizeof(prop),
    2484                                "mediatek,%s-multi-pin-mode",
    2485                                of_afe_etdms[i].name);
    2486                 if (ret < 0) {
--> 2487                         dev_err(afe->dev, "%s snprintf err=%d\n",
    2488                                 __func__, ret);


I guess the reason behind this warning is that we checked something but
we didn't check if "ret >= sizeof(prop)" as one would normally suspect.

In user space snprintf() can return negatives but in the kernel it never
does.  Changing that in the future is impossible because it would
introduce a ton of security issues, so it never will.

I guess I suggest just removing the dead code.

    2489                         return;
    2490                 }
    2491                 etdm_data->data_mode = of_property_read_bool(of_node, prop);
    2492 
    2493                 ret = snprintf(prop, sizeof(prop),
    2494                                "mediatek,%s-cowork-source",
    2495                                of_afe_etdms[i].name);
    2496                 if (ret < 0) {

Same

    2497                         dev_err(afe->dev, "%s snprintf err=%d\n",
    2498                                 __func__, ret);
    2499                         return;
    2500                 }
    2501                 ret = of_property_read_u32(of_node, prop, &sel);
    2502                 if (ret == 0) {
    2503                         if (sel >= MT8188_AFE_IO_ETDM_NUM) {
    2504                                 dev_err(afe->dev, "%s invalid id=%d\n",
    2505                                         __func__, sel);
    2506                                 etdm_data->cowork_source_id = COWORK_ETDM_NONE;
    2507                         } else {
    2508                                 sync_id = of_afe_etdms[sel].sync_id;
    2509                                 etdm_data->cowork_source_id =
    2510                                         sync_to_dai_id(sync_id);
    2511                         }
    2512                 } else {
    2513                         etdm_data->cowork_source_id = COWORK_ETDM_NONE;
    2514                 }
    2515         }
    2516 
    2517         /* etdm in only */
    2518         for (i = 0; i < 2; i++) {
    2519                 ret = snprintf(prop, sizeof(prop),
    2520                                "mediatek,%s-chn-disabled",
    2521                                of_afe_etdms[i].name);
    2522                 if (ret < 0) {

Same.

    2523                         dev_err(afe->dev, "%s snprintf err=%d\n",
    2524                                 __func__, ret);
    2525                         return;
    2526                 }
    2527                 ret = of_property_read_variable_u8_array(of_node, prop,
    2528                                                          disable_chn,
    2529                                                          1, max_chn);
    2530                 if (ret < 0)
    2531                         continue;
    2532 
    2533                 for (j = 0; j < ret; j++) {
    2534                         if (disable_chn[j] >= MT8188_ETDM_MAX_CHANNELS)
    2535                                 dev_err(afe->dev, "%s [%d] invalid chn %u\n",
    2536                                         __func__, j, disable_chn[j]);
    2537                         else
    2538                                 etdm_data->in_disable_ch[disable_chn[j]] = true;
    2539                 }
    2540         }
    2541         mt8188_etdm_update_sync_info(afe);
    2542 }

regards,
dan carpenter



More information about the Linux-mediatek mailing list