[Public]
Applied. Thanks!
________________________________
From: Wentland, Harry <harry.wentl...@amd.com>
Sent: Friday, January 21, 2022 2:03 PM
To: Yongzhi Liu <lyz...@pku.edu.cn>; Li, Sun peng (Leo) <sunpeng...@amd.com>;
Siqueira, Rodrigo <rodrigo.sique...@amd.com>; Deucher, Alexander
<alexander.deuc...@amd.com>; Koenig, Christian <christian.koe...@amd.com>; Pan,
Xinhui <xinhui....@amd.com>; airl...@linux.ie <airl...@linux.ie>;
dan...@ffwll.ch <dan...@ffwll.ch>; Lipski, Mikita <mikita.lip...@amd.com>; Lin,
Wayne <wayne....@amd.com>; Kazlauskas, Nicholas <nicholas.kazlaus...@amd.com>;
Zuo, Jerry <jerry....@amd.com>; anson.ja...@amd.com <anson.ja...@amd.com>;
eryk.b...@amd.com <eryk.b...@amd.com>; Pillai, Aurabindo
<aurabindo.pil...@amd.com>; Das, Nirmoy <nirmoy....@amd.com>
Cc: amd-...@lists.freedesktop.org <amd-...@lists.freedesktop.org>;
dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org>;
linux-ker...@vger.kernel.org <linux-ker...@vger.kernel.org>
Subject: Re: [PATCH] drm/amd/display: Fix memory leak
On 2022-01-21 06:26, Yongzhi Liu wrote:
> [why]
> Resource release is needed on the error handling path
> to prevent memory leak.
>
> [how]
> Fix this by adding kfree on the error handling path.
>
> Signed-off-by: Yongzhi Liu <lyz...@pku.edu.cn>
Reviewed-by: Harry Wentland <harry.wentl...@amd.com>
Harry
> ---
> .../drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 80
> ++++++++++++++++------
> 1 file changed, 60 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> index ded64d0..e463d46 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> @@ -227,8 +227,10 @@ static ssize_t dp_link_settings_read(struct file *f,
> char __user *buf,
> break;
>
> r = put_user(*(rd_buf + result), buf);
> - if (r)
> + if (r) {
> + kfree(rd_buf);
> return r; /* r = -EFAULT */
> + }
>
> buf += 1;
> size -= 1;
> @@ -389,8 +391,10 @@ static ssize_t dp_phy_settings_read(struct file *f, char
> __user *buf,
> break;
>
> r = put_user((*(rd_buf + result)), buf);
> - if (r)
> + if (r) {
> + kfree(rd_buf);
> return r; /* r = -EFAULT */
> + }
>
> buf += 1;
> size -= 1;
> @@ -1359,8 +1363,10 @@ static ssize_t dp_dsc_clock_en_read(struct file *f,
> char __user *buf,
> break;
> }
>
> - if (!pipe_ctx)
> + if (!pipe_ctx) {
> + kfree(rd_buf);
> return -ENXIO;
> + }
>
> dsc = pipe_ctx->stream_res.dsc;
> if (dsc)
> @@ -1376,8 +1382,10 @@ static ssize_t dp_dsc_clock_en_read(struct file *f,
> char __user *buf,
> break;
>
> r = put_user(*(rd_buf + result), buf);
> - if (r)
> + if (r) {
> + kfree(rd_buf);
> return r; /* r = -EFAULT */
> + }
>
> buf += 1;
> size -= 1;
> @@ -1546,8 +1554,10 @@ static ssize_t dp_dsc_slice_width_read(struct file *f,
> char __user *buf,
> break;
> }
>
> - if (!pipe_ctx)
> + if (!pipe_ctx) {
> + kfree(rd_buf);
> return -ENXIO;
> + }
>
> dsc = pipe_ctx->stream_res.dsc;
> if (dsc)
> @@ -1563,8 +1573,10 @@ static ssize_t dp_dsc_slice_width_read(struct file *f,
> char __user *buf,
> break;
>
> r = put_user(*(rd_buf + result), buf);
> - if (r)
> + if (r) {
> + kfree(rd_buf);
> return r; /* r = -EFAULT */
> + }
>
> buf += 1;
> size -= 1;
> @@ -1731,8 +1743,10 @@ static ssize_t dp_dsc_slice_height_read(struct file
> *f, char __user *buf,
> break;
> }
>
> - if (!pipe_ctx)
> + if (!pipe_ctx) {
> + kfree(rd_buf);
> return -ENXIO;
> + }
>
> dsc = pipe_ctx->stream_res.dsc;
> if (dsc)
> @@ -1748,8 +1762,10 @@ static ssize_t dp_dsc_slice_height_read(struct file
> *f, char __user *buf,
> break;
>
> r = put_user(*(rd_buf + result), buf);
> - if (r)
> + if (r) {
> + kfree(rd_buf);
> return r; /* r = -EFAULT */
> + }
>
> buf += 1;
> size -= 1;
> @@ -1912,8 +1928,10 @@ static ssize_t dp_dsc_bits_per_pixel_read(struct file
> *f, char __user *buf,
> break;
> }
>
> - if (!pipe_ctx)
> + if (!pipe_ctx) {
> + kfree(rd_buf);
> return -ENXIO;
> + }
>
> dsc = pipe_ctx->stream_res.dsc;
> if (dsc)
> @@ -1929,8 +1947,10 @@ static ssize_t dp_dsc_bits_per_pixel_read(struct file
> *f, char __user *buf,
> break;
>
> r = put_user(*(rd_buf + result), buf);
> - if (r)
> + if (r) {
> + kfree(rd_buf);
> return r; /* r = -EFAULT */
> + }
>
> buf += 1;
> size -= 1;
> @@ -2088,8 +2108,10 @@ static ssize_t dp_dsc_pic_width_read(struct file *f,
> char __user *buf,
> break;
> }
>
> - if (!pipe_ctx)
> + if (!pipe_ctx) {
> + kfree(rd_buf);
> return -ENXIO;
> + }
>
> dsc = pipe_ctx->stream_res.dsc;
> if (dsc)
> @@ -2105,8 +2127,10 @@ static ssize_t dp_dsc_pic_width_read(struct file *f,
> char __user *buf,
> break;
>
> r = put_user(*(rd_buf + result), buf);
> - if (r)
> + if (r) {
> + kfree(rd_buf);
> return r; /* r = -EFAULT */
> + }
>
> buf += 1;
> size -= 1;
> @@ -2145,8 +2169,10 @@ static ssize_t dp_dsc_pic_height_read(struct file *f,
> char __user *buf,
> break;
> }
>
> - if (!pipe_ctx)
> + if (!pipe_ctx) {
> + kfree(rd_buf);
> return -ENXIO;
> + }
>
> dsc = pipe_ctx->stream_res.dsc;
> if (dsc)
> @@ -2162,8 +2188,10 @@ static ssize_t dp_dsc_pic_height_read(struct file *f,
> char __user *buf,
> break;
>
> r = put_user(*(rd_buf + result), buf);
> - if (r)
> + if (r) {
> + kfree(rd_buf);
> return r; /* r = -EFAULT */
> + }
>
> buf += 1;
> size -= 1;
> @@ -2217,8 +2245,10 @@ static ssize_t dp_dsc_chunk_size_read(struct file *f,
> char __user *buf,
> break;
> }
>
> - if (!pipe_ctx)
> + if (!pipe_ctx) {
> + kfree(rd_buf);
> return -ENXIO;
> + }
>
> dsc = pipe_ctx->stream_res.dsc;
> if (dsc)
> @@ -2234,8 +2264,10 @@ static ssize_t dp_dsc_chunk_size_read(struct file *f,
> char __user *buf,
> break;
>
> r = put_user(*(rd_buf + result), buf);
> - if (r)
> + if (r) {
> + kfree(rd_buf);
> return r; /* r = -EFAULT */
> + }
>
> buf += 1;
> size -= 1;
> @@ -2289,8 +2321,10 @@ static ssize_t dp_dsc_slice_bpg_offset_read(struct
> file *f, char __user *buf,
> break;
> }
>
> - if (!pipe_ctx)
> + if (!pipe_ctx) {
> + kfree(rd_buf);
> return -ENXIO;
> + }
>
> dsc = pipe_ctx->stream_res.dsc;
> if (dsc)
> @@ -2306,8 +2340,10 @@ static ssize_t dp_dsc_slice_bpg_offset_read(struct
> file *f, char __user *buf,
> break;
>
> r = put_user(*(rd_buf + result), buf);
> - if (r)
> + if (r) {
> + kfree(rd_buf);
> return r; /* r = -EFAULT */
> + }
>
> buf += 1;
> size -= 1;
> @@ -3459,8 +3495,10 @@ static ssize_t dcc_en_bits_read(
> dc->hwss.get_dcc_en_bits(dc, dcc_en_bits);
>
> rd_buf = kcalloc(rd_buf_size, sizeof(char), GFP_KERNEL);
> - if (!rd_buf)
> + if (!rd_buf) {
> + kfree(dcc_en_bits);
> return -ENOMEM;
> + }
>
> for (i = 0; i < num_pipes; i++)
> offset += snprintf(rd_buf + offset, rd_buf_size - offset,
> @@ -3473,8 +3511,10 @@ static ssize_t dcc_en_bits_read(
> if (*pos >= rd_buf_size)
> break;
> r = put_user(*(rd_buf + result), buf);
> - if (r)
> + if (r) {
> + kfree(rd_buf);
> return r; /* r = -EFAULT */
> + }
> buf += 1;
> size -= 1;
> *pos += 1;
