[PATCH 1/2] drm: not to read outside the boundary for CRC source name.

Sam Ravnborg sam at ravnborg.org
Wed Jun 5 17:50:03 UTC 2019


Hi Dingchen.

Thanks for the patch!

On Wed, Jun 05, 2019 at 01:06:38PM -0400, Dingchen Zhang wrote:
> 'n-1' is the index to access the last character of CRC source name.
> 
> Cc:Leo Li <sunpeng.li at amd.com>, Harry Wentland<Harry.Wentland at amd.com>
Please add only one person (mail address) per Cc: line

I dunno if this is a hard rule, but this is what we always do.

> Signed-off-by: Dingchen Zhang <dingchen.zhang at amd.com>
> ---
>  drivers/gpu/drm/drm_debugfs_crc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
> index 585169f0dcc5..e20adef9d623 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -131,8 +131,8 @@ static ssize_t crc_control_write(struct file *file, const char __user *ubuf,
>  	if (IS_ERR(source))
>  		return PTR_ERR(source);
>  
> -	if (source[len] == '\n')
> -		source[len] = '\0';
> +	if (source[len-1] == '\n')
> +		source[len-1] = '\0';
In the kernel code we add spaces around operators.
So the above should be:  source[len - 1]

Details aside.
memdup_user_nul() which is called gurantee that the buffer is null
terminated. The buffer allocated is len + 1 and the last byte in the
buffer is set to '\0' in memdup_user_nul().

So the right fix is to kill the two lines since they have no effect.
Could you please verify my analysis, and if you agree submit a new
patch.

Thanks,
	Sam


More information about the dri-devel mailing list