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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hey Dan,

On 10/1/2021 6:59 AM, 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).

Recently, Bjorn Andersson released a patch that removes copy_to_user() from this method (https://patchwork.freedesktop.org/patch/457978/). It's been added to msm-next-staging. Can you retest with this patch?

Thanks,

Jessica Zhang

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



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux