Date:   Tue, 14 Mar 2023 13:09:04 +0200
From:   Jarkko Sakkinen <jarkko@kernel.org>
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     linux-kernel@vger.kernel.org, rafael@kernel.org,
        Peter Huewe <peterhuewe@gmx.de>,
        Jason Gunthorpe <jgg@ziepe.ca>, linux-integrity@vger.kernel.org
Subject: Re: [PATCH 10/12] tpm: fix up the tpm_class shutdown_pre pointer
 when created
Message-ID: <ZBBV0N+eHSf2TYli@kernel.org>
References: <20230313181843.1207845-1-gregkh@linuxfoundation.org>
 <20230313181843.1207845-10-gregkh@linuxfoundation.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230313181843.1207845-10-gregkh@linuxfoundation.org>
Precedence: bulk

On Mon, Mar 13, 2023 at 07:18:41PM +0100, Greg Kroah-Hartman wrote:
> Do not wait until long after the struct class has been created to set
> the shutdown_pre pointer for the tpm_class, assign it right away.
> 
> This is the only in-kernel offender that tries to modify the
> device->class pointer contents after it has been assigned to a device,
> so fix that up by doing the function pointer assignment before it is
> matched with the device.  Because of this, the patch should go through
> the driver core tree to allow later changes to struct device to be
> possible.
> 
> Cc: Peter Huewe <peterhuewe@gmx.de>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: linux-integrity@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/char/tpm/tpm-chip.c      | 3 +--
>  drivers/char/tpm/tpm-interface.c | 1 +
>  drivers/char/tpm/tpm.h           | 1 +
>  3 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index b99f55f2d4fd..7c444209a256 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -282,7 +282,7 @@ static void tpm_dev_release(struct device *dev)
>   *
>   * Return: always 0 (i.e. success)
>   */
> -static int tpm_class_shutdown(struct device *dev)
> +int tpm_class_shutdown(struct device *dev)
>  {
>  	struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev);
>  
> @@ -337,7 +337,6 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>  	device_initialize(&chip->dev);
>  
>  	chip->dev.class = tpm_class;
> -	chip->dev.class->shutdown_pre = tpm_class_shutdown;
>  	chip->dev.release = tpm_dev_release;
>  	chip->dev.parent = pdev;
>  	chip->dev.groups = chip->groups;
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 8763c820d1f8..43e23a04433a 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -467,6 +467,7 @@ static int __init tpm_init(void)
>  	int rc;
>  
>  	tpm_class = class_create("tpm");
> +	tpm_class->shutdown_pre = tpm_class_shutdown;
>  	if (IS_ERR(tpm_class)) {
>  		pr_err("couldn't create tpm class\n");
>  		return PTR_ERR(tpm_class);
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 24ee4e1cc452..a45eb39db0c4 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -183,6 +183,7 @@ int tpm1_get_pcr_allocation(struct tpm_chip *chip);
>  unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
>  int tpm_pm_suspend(struct device *dev);
>  int tpm_pm_resume(struct device *dev);
> +int tpm_class_shutdown(struct device *dev);
>  
>  static inline void tpm_msleep(unsigned int delay_msec)
>  {
> -- 
> 2.39.2
> 

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Are you going to pick this?

BR, Jarkko