[bug report] drm/msm/dp: add debugfs support to DP driver

jesszhan at codeaurora.org jesszhan at codeaurora.org
Fri Oct 1 19:00:03 UTC 2021


Hey Dan,

Thanks for the heads up, will take care of it.

On 2021-10-01 06:59, Dan Carpenter wrote:
> Hello Abhinav Kumar,
> 
> The patch d11a93690df7: "drm/msm/dp: add debugfs support to DP
> driver" from Sep 12, 2020, leads to the following
> Smatch static checker warning:
> 
> 	drivers/gpu/drm/msm/dp/dp_debug.c:194 dp_debug_read_info()
> 	warn: userbuf overflow? is 'len' <= 'count'
> 
> drivers/gpu/drm/msm/dp/dp_debug.c
>     46 static ssize_t dp_debug_read_info(struct file *file, char
> __user *user_buff,
>     47                 size_t count, loff_t *ppos)
>     48 {
>     49         struct dp_debug_private *debug = file->private_data;
>     50         char *buf;
>     51         u32 len = 0, rc = 0;
>     52         u64 lclk = 0;
>     53         u32 max_size = SZ_4K;
>     54         u32 link_params_rate;
>     55         struct drm_display_mode *drm_mode;
>     56
>     57         if (!debug)
>     58                 return -ENODEV;
>     59
>     60         if (*ppos)
>     61                 return 0;
>     62
>     63         buf = kzalloc(SZ_4K, GFP_KERNEL);
>     64         if (!buf)
>     65                 return -ENOMEM;
>     66
>     67         drm_mode = &debug->panel->dp_mode.drm_mode;
>     68
>     69         rc = snprintf(buf + len, max_size, "\tname = %s\n", 
> DEBUG_NAME);
>     70         if (dp_debug_check_buffer_overflow(rc, &max_size, &len))
>     71                 goto error;
>     72
>     73         rc = snprintf(buf + len, max_size,
>     74                         "\tdp_panel\n\t\tmax_pclk_khz = %d\n",
>     75                         debug->panel->max_pclk_khz);
>     76         if (dp_debug_check_buffer_overflow(rc, &max_size, &len))
>     77                 goto error;
>     78
>     79         rc = snprintf(buf + len, max_size,
>     80                         "\tdrm_dp_link\n\t\trate = %u\n",
>     81                         debug->panel->link_info.rate);
>     82         if (dp_debug_check_buffer_overflow(rc, &max_size, &len))
>     83                 goto error;
>     84
>     85         rc = snprintf(buf + len, max_size,
>     86                          "\t\tnum_lanes = %u\n",
>     87                         debug->panel->link_info.num_lanes);
>     88         if (dp_debug_check_buffer_overflow(rc, &max_size, &len))
>     89                 goto error;
>     90
>     91         rc = snprintf(buf + len, max_size,
>     92                         "\t\tcapabilities = %lu\n",
>     93                         debug->panel->link_info.capabilities);
>     94         if (dp_debug_check_buffer_overflow(rc, &max_size, &len))
>     95                 goto error;
>     96
>     97         rc = snprintf(buf + len, max_size,
>     98                         "\tdp_panel_info:\n\t\tactive = 
> %dx%d\n",
>     99                         drm_mode->hdisplay,
>     100                         drm_mode->vdisplay);
>     101         if (dp_debug_check_buffer_overflow(rc, &max_size, 
> &len))
>     102                 goto error;
>     103
>     104         rc = snprintf(buf + len, max_size,
>     105                         "\t\tback_porch = %dx%d\n",
>     106                         drm_mode->htotal - drm_mode->hsync_end,
>     107                         drm_mode->vtotal - 
> drm_mode->vsync_end);
>     108         if (dp_debug_check_buffer_overflow(rc, &max_size, 
> &len))
>     109                 goto error;
>     110
>     111         rc = snprintf(buf + len, max_size,
>     112                         "\t\tfront_porch = %dx%d\n",
>     113                         drm_mode->hsync_start - 
> drm_mode->hdisplay,
>     114                         drm_mode->vsync_start - 
> drm_mode->vdisplay);
>     115         if (dp_debug_check_buffer_overflow(rc, &max_size, 
> &len))
>     116                 goto error;
>     117
>     118         rc = snprintf(buf + len, max_size,
>     119                         "\t\tsync_width = %dx%d\n",
>     120                         drm_mode->hsync_end - 
> drm_mode->hsync_start,
>     121                         drm_mode->vsync_end - 
> drm_mode->vsync_start);
>     122         if (dp_debug_check_buffer_overflow(rc, &max_size, 
> &len))
>     123                 goto error;
>     124
>     125         rc = snprintf(buf + len, max_size,
>     126                         "\t\tactive_low = %dx%d\n",
>     127                         debug->panel->dp_mode.h_active_low,
>     128                         debug->panel->dp_mode.v_active_low);
>     129         if (dp_debug_check_buffer_overflow(rc, &max_size, 
> &len))
>     130                 goto error;
>     131
>     132         rc = snprintf(buf + len, max_size,
>     133                         "\t\th_skew = %d\n",
>     134                         drm_mode->hskew);
>     135         if (dp_debug_check_buffer_overflow(rc, &max_size, 
> &len))
>     136                 goto error;
>     137
>     138         rc = snprintf(buf + len, max_size,
>     139                         "\t\trefresh rate = %d\n",
>     140                         drm_mode_vrefresh(drm_mode));
>     141         if (dp_debug_check_buffer_overflow(rc, &max_size, 
> &len))
>     142                 goto error;
>     143
>     144         rc = snprintf(buf + len, max_size,
>     145                         "\t\tpixel clock khz = %d\n",
>     146                         drm_mode->clock);
>     147         if (dp_debug_check_buffer_overflow(rc, &max_size, 
> &len))
>     148                 goto error;
>     149
>     150         rc = snprintf(buf + len, max_size,
>     151                         "\t\tbpp = %d\n",
>     152                         debug->panel->dp_mode.bpp);
>     153         if (dp_debug_check_buffer_overflow(rc, &max_size, 
> &len))
>     154                 goto error;
>     155
>     156         /* Link Information */
>     157         rc = snprintf(buf + len, max_size,
>     158                         "\tdp_link:\n\t\ttest_requested = 
> %d\n",
>     159                         debug->link->sink_request);
>     160         if (dp_debug_check_buffer_overflow(rc, &max_size, 
> &len))
>     161                 goto error;
>     162
>     163         rc = snprintf(buf + len, max_size,
>     164                         "\t\tnum_lanes = %d\n",
>     165                         debug->link->link_params.num_lanes);
>     166         if (dp_debug_check_buffer_overflow(rc, &max_size, 
> &len))
>     167                 goto error;
>     168
>     169         link_params_rate = debug->link->link_params.rate;
>     170         rc = snprintf(buf + len, max_size,
>     171                         "\t\tbw_code = %d\n",
>     172                         
> drm_dp_link_rate_to_bw_code(link_params_rate));
>     173         if (dp_debug_check_buffer_overflow(rc, &max_size, 
> &len))
>     174                 goto error;
>     175
>     176         lclk = debug->link->link_params.rate * 1000;
>     177         rc = snprintf(buf + len, max_size,
>     178                         "\t\tlclk = %lld\n", lclk);
>     179         if (dp_debug_check_buffer_overflow(rc, &max_size, 
> &len))
>     180                 goto error;
>     181
>     182         rc = snprintf(buf + len, max_size,
>     183                         "\t\tv_level = %d\n",
>     184                         debug->link->phy_params.v_level);
>     185         if (dp_debug_check_buffer_overflow(rc, &max_size, 
> &len))
>     186                 goto error;
>     187
>     188         rc = snprintf(buf + len, max_size,
>     189                         "\t\tp_level = %d\n",
>     190                         debug->link->phy_params.p_level);
>     191         if (dp_debug_check_buffer_overflow(rc, &max_size, 
> &len))
>     192                 goto error;
>     193
> --> 194         if (copy_to_user(user_buff, buf, len))
> 
> This function does not take "count" into consideration so it can end
> up copying more than the user wanted (memory corruption in the user
> program).
> 
> Technically if copy_to_user() fails it should return -EFAULT, not 
> -EINVAL.
> 
> It's weird how it return -EINVAL when the kernel can't fit its output
> in one page.  Normally we would just print the first page and hope that
> was enough.  Use scprintf() instead snprintf().
> 
> 	len += scnprintf(buf + len, size - len, "blah blah blah");
> 
>     195                 goto error;
>     196
>     197         *ppos += len;
>     198
>     199         kfree(buf);
>     200         return len;
>     201  error:
>     202         kfree(buf);
>     203         return -EINVAL;
>     204 }
> 
> regards,
> dan carpenter

Best,
Jessica Zhang


More information about the dri-devel mailing list