Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <558d22a8848f2f026f272fdc0528424d.sboyd@kernel.org>
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <20230413-creamer-overstate-f7ce5a72e437@spud>
References: <20230413205528.4044216-1-sboyd@kernel.org> <20230413-creamer-overstate-f7ce5a72e437@spud>
Subject: Re: [PATCH] clk: starfive: Avoid casting iomem pointers
From:   Stephen Boyd <sboyd@kernel.org>
Cc:     Michael Turquette <mturquette@baylibre.com>,
        linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
        patches@lists.linux.dev, Tommaso Merciai <tomm.merciai@gmail.com>,
        Emil Renner Berthing <emil.renner.berthing@canonical.com>,
        Hal Feng <hal.feng@starfivetech.com>,
        Conor Dooley <conor.dooley@microchip.com>,
        Xingyu Wu <xingyu.wu@starfivetech.com>
To:     Conor Dooley <conor@kernel.org>
Date:   Thu, 13 Apr 2023 15:01:02 -0700
User-Agent: alot/0.10
Precedence: bulk

Quoting Conor Dooley (2023-04-13 14:26:56)
> > @@ -335,26 +338,32 @@ static void jh7110_reset_unregister_adev(void *_a=
dev)
> >       struct auxiliary_device *adev =3D _adev;
> > =20
> >       auxiliary_device_delete(adev);
> > +     auxiliary_device_uninit(adev);
>=20
> Huh, I think you didn't explicitly mention this one, but it's actually
> part of the UAF fix AFAICT?
>=20
> When I did the aux device stuff for the clk-mpfs driver, I copied from
> peci as there were almost no examples of aux dev stuff in-tree.
> It looks like subsequently to me starting development, this fix landed:
> 1c11289b34ab ("peci: cpu: Fix use-after-free in adev_release()")
>=20
> It similarly moves the uninit() to the release callback...
>=20
> I think I need the below (whitespace damaged):

Yeah that looks better. Care to send a proper patch for it?

> diff --git a/drivers/clk/microchip/clk-mpfs.c b/drivers/clk/microchip/clk=
-mpfs.c
> index 4f0a19db7ed7..cc5d7dee59f0 100644
> --- a/drivers/clk/microchip/clk-mpfs.c
> +++ b/drivers/clk/microchip/clk-mpfs.c
> @@ -374,14 +374,13 @@ static void mpfs_reset_unregister_adev(void *_adev)
>         struct auxiliary_device *adev =3D _adev;
> =20
>         auxiliary_device_delete(adev);
> +       auxiliary_device_uninit(adev);
>  }
> =20
>  static void mpfs_reset_adev_release(struct device *dev)
>  {
>         struct auxiliary_device *adev =3D to_auxiliary_dev(dev);
> =20
> -       auxiliary_device_uninit(adev);
> -
>         kfree(adev);
>  }
>=20
> Anyways, for this patch:
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>=20

Thanks.