On Tue, Jan 10, 2023 at 1:29 PM Rob Clark <robdclark@xxxxxxxxx> wrote:
>
> From: Rob Clark <robdclark@xxxxxxxxxxxx>
>
> If userspace was calling the MSM_SET_PARAM ioctl on multiple threads to
> set the COMM or CMDLINE param, it could trigger a race causing the
> previous value to be kfree'd multiple times. Fix this by serializing on
> the gpu lock.
>
Fixes: d4726d770068 ("drm/msm: Add a way to override processes comm/cmdline")
> Signed-off-by: Rob Clark <robdclark@xxxxxxxxxxxx>
> ---
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 4 ++++
> drivers/gpu/drm/msm/msm_gpu.c | 2 ++
> drivers/gpu/drm/msm/msm_gpu.h | 12 ++++++++++--
> 3 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 57586c794b84..3605f095b2de 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -352,6 +352,8 @@ int adreno_set_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
> /* Ensure string is null terminated: */
> str[len] = '\0';
>
> + mutex_lock(&gpu->lock);
> +
> if (param == MSM_PARAM_COMM) {
> paramp = &ctx->comm;
> } else {
> @@ -361,6 +363,8 @@ int adreno_set_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
> kfree(*paramp);
> *paramp = str;
>
> + mutex_unlock(&gpu->lock);
> +
> return 0;
> }
> case MSM_PARAM_SYSPROF:
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index bfef659d3a5c..7537e7b3a452 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -336,6 +336,8 @@ static void get_comm_cmdline(struct msm_gem_submit *submit, char **comm, char **
> struct msm_file_private *ctx = submit->queue->ctx;
> struct task_struct *task;
>
> + WARN_ON(!mutex_is_locked(&submit->gpu->lock));
> +
> /* Note that kstrdup will return NULL if argument is NULL: */
> *comm = kstrdup(ctx->comm, GFP_KERNEL);
> *cmd = kstrdup(ctx->cmdline, GFP_KERNEL);
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index a771f56ed70f..fc1c0d8611a8 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -375,10 +375,18 @@ struct msm_file_private {
> */
> int sysprof;
>
> - /** comm: Overridden task comm, see MSM_PARAM_COMM */
> + /**
> + * comm: Overridden task comm, see MSM_PARAM_COMM
> + *
> + * Accessed under msm_gpu::lock
> + */
> char *comm;
>
> - /** cmdline: Overridden task cmdline, see MSM_PARAM_CMDLINE */
> + /**
> + * cmdline: Overridden task cmdline, see MSM_PARAM_CMDLINE
> + *
> + * Accessed under msm_gpu::lock
> + */
> char *cmdline;
>
> /**
> --
> 2.38.1
>
