From: jesszhan@codeaurora.org
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: abhinavk@codeaurora.org, 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: Fri, 01 Oct 2021 12:00:03 -0700 [thread overview]
Message-ID: <934251d1ee057a0bd745d10265425980@codeaurora.org> (raw)
In-Reply-To: <20211001135937.GA10064@kili>
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
next prev parent reply other threads:[~2021-10-01 19:01 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 [this message]
2021-10-19 17:56 ` Jessica Zhang
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=934251d1ee057a0bd745d10265425980@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).