2023-04-25 13:33:31

by Robert Foss

[permalink] [raw]
Subject: Re: [PATCH] drm/bridge: it6505: Move a variable assignment behind a null pointer check in receive_timing_debugfs_show()

Hey Markus,

This patch seems to be a part of a series without being marked as
such, this causes issues when importing this patch with maintainer
tools like b4 which automatically pull in the entire series and not
just the specific patch. Either label the patch as being part of a
series ( [PATCH 1/XX] ), or submit it separately.

On Sun, Apr 16, 2023 at 5:47 PM Markus Elfring <[email protected]> wrote:
>
> Date: Sun, 16 Apr 2023 17:30:46 +0200
>
> The address of a data structure member was determined before
> a corresponding null pointer check in the implementation of
> the function “receive_timing_debugfs_show”.
>
> Thus avoid the risk for undefined behaviour by moving the assignment
> for the variable “vid” behind the null pointer check.
>
> This issue was detected by using the Coccinelle software.
>
> Fixes: b5c84a9edcd418cd055becad6a22439e7c5e3bf8 ("drm/bridge: add it6505 driver")
> Signed-off-by: Markus Elfring <[email protected]>

The email in the Signed-off tag should match the email of the sender,
which it doesn't.

With the two above issues fixed, please add my r-b.
Reviewed-by: Robert Foss <[email protected]>

> ---
> drivers/gpu/drm/bridge/ite-it6505.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index abaf6e23775e..45f579c365e7 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -3207,7 +3207,7 @@ static ssize_t receive_timing_debugfs_show(struct file *file, char __user *buf,
> size_t len, loff_t *ppos)
> {
> struct it6505 *it6505 = file->private_data;
> - struct drm_display_mode *vid = &it6505->video_info;
> + struct drm_display_mode *vid;
> u8 read_buf[READ_BUFFER_SIZE];
> u8 *str = read_buf, *end = read_buf + READ_BUFFER_SIZE;
> ssize_t ret, count;
> @@ -3216,6 +3216,7 @@ static ssize_t receive_timing_debugfs_show(struct file *file, char __user *buf,
> return -ENODEV;
>
> it6505_calc_video_info(it6505);
> + vid = &it6505->video_info;
> str += scnprintf(str, end - str, "---video timing---\n");
> str += scnprintf(str, end - str, "PCLK:%d.%03dMHz\n",
> vid->clock / 1000, vid->clock % 1000);
> --
> 2.40.0
>