On Wed, Mar 16, 2022 at 05:29:45PM -0700, Rob Clark wrote:
> switch (param) {
> + case MSM_PARAM_COMM:
> + case MSM_PARAM_CMDLINE: {
> + char *str, **paramp;
> +
> + str = kmalloc(len + 1, GFP_KERNEL);
if (!str)
return -ENOMEM;
> + if (copy_from_user(str, u64_to_user_ptr(value), len)) {
> + kfree(str);
> + return -EFAULT;
> + }
> +
> + /* Ensure string is null terminated: */
> + str[len] = '\0';
> +
> + if (param == MSM_PARAM_COMM) {
> + paramp = &ctx->comm;
> + } else {
> + paramp = &ctx->cmdline;
> + }
> +
> + kfree(*paramp);
> + *paramp = str;
> +
> + return 0;
> + }
> case MSM_PARAM_SYSPROF:
> if (!capable(CAP_SYS_ADMIN))
> return -EPERM;
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 4ec62b601adc..68f3f8ade76d 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -364,14 +364,21 @@ static void retire_submits(struct msm_gpu *gpu);
>
> static void get_comm_cmdline(struct msm_gem_submit *submit, char **comm, char **cmd)
> {
> + struct msm_file_private *ctx = submit->queue->ctx;
> struct task_struct *task;
>
> + *comm = kstrdup(ctx->comm, GFP_KERNEL);
> + *cmd = kstrdup(ctx->cmdline, GFP_KERNEL);
> +
> task = get_pid_task(submit->pid, PIDTYPE_PID);
> if (!task)
> return;
>
> - *comm = kstrdup(task->comm, GFP_KERNEL);
> - *cmd = kstrdup_quotable_cmdline(task, GFP_KERNEL);
> + if (!*comm)
> + *comm = kstrdup(task->comm, GFP_KERNEL);
What?
If the first allocation failed, then this one is going to fail as well.
Just return -ENOMEM. Or maybe this is meant to be checking for an empty
string?
> +
> + if (!*cmd)
> + *cmd = kstrdup_quotable_cmdline(task, GFP_KERNEL);
Same.
>
> put_task_struct(task);
> }
regards,
dan carpenter
