Re: [PATCH 3/3] drm/msm: Add a way to override processes comm/cmdline

[Dan Carpenter]

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