From: Jessica Zhang <jesszhan@codeaurora.org>
To: Dan Carpenter <dan.carpenter@oracle.com>, abhinavk@codeaurora.org
Cc: linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [bug report] drm/msm/dp: add debugfs support to DP driver
Date: Tue, 19 Oct 2021 10:56:57 -0700 [thread overview]
Message-ID: <fa2be3e2-5857-6ca4-e81b-39f4aea2b399@codeaurora.org> (raw)
In-Reply-To: <20211001135937.GA10064@kili>
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
prev parent reply other threads:[~2021-10-19 17:57 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-01 13:59 [bug report] drm/msm/dp: add debugfs support to DP driver Dan Carpenter
2021-10-01 19:00 ` jesszhan
2021-10-19 17:56 ` Jessica Zhang [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fa2be3e2-5857-6ca4-e81b-39f4aea2b399@codeaurora.org \
--to=jesszhan@codeaurora.org \
--cc=abhinavk@codeaurora.org \
--cc=dan.carpenter@oracle.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-arm-msm@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.