linux-kernel - Re: [PATCH] drm/msm/gpu: Fix potential double-free

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAF6AEGsivdz-e+sCRQto1_WBZ80bv8b4xDbYvzX-4sVo9kM7Yw@mail.gmail.com>
Date:   Wed, 11 Jan 2023 07:28:45 -0800
From:   Rob Clark <robdclark@...il.com>
To:     dri-devel@...ts.freedesktop.org
Cc:     freedreno@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
        Rob Clark <robdclark@...omium.org>,
        Abhinav Kumar <quic_abhinavk@...cinc.com>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Sean Paul <sean@...rly.run>, David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Akhil P Oommen <quic_akhilpo@...cinc.com>,
        Chia-I Wu <olvaffe@...il.com>, Emma Anholt <emma@...olt.net>,
        Dan Carpenter <error27@...il.com>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/msm/gpu: Fix potential double-free

On Tue, Jan 10, 2023 at 1:29 PM Rob Clark <robdclark@...il.com> wrote:
>
> From: Rob Clark <robdclark@...omium.org>
>
> 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@...omium.org>
> ---
>  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
>