Received-SPF: pass (google.com: domain of linux-kernel+bounces-149335-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240417230637.2592473-1-dtokazaki@google.com>
In-Reply-To: <20240417230637.2592473-1-dtokazaki@google.com>
From: Bartosz Golaszewski <brgl@bgdev.pl>
Date: Thu, 18 Apr 2024 01:59:46 +0200
Message-ID: <CAMRc=Mcbi8BQ-tHNs+BVjbm3aDSVY0zzOM4QfND3d2y8ryT62g@mail.gmail.com>
Subject: Re: [PATCH v1] at24: fix memory corruption race condition
To: Daniel Okazaki <dtokazaki@google.com>
Cc: Arnd Bergmann <arnd@arndb.de>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, kernel-team@android.com, 
	linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Apr 18, 2024 at 1:07=E2=80=AFAM Daniel Okazaki <dtokazaki@google.co=
m> wrote:
>
> If the eeprom is not accessible, an nvmem device will be registered, the
> read will fail, and the device will be torn down. If another driver
> accesses the nvmem device after the teardown, it will reference
> invalid memory.
>
> Move the failure point before registering the nvmem device.
>
> Signed-off-by: Daniel Okazaki <dtokazaki@google.com>
> ---
>  drivers/misc/eeprom/at24.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 572333ead5fb..4bd4f32bcdab 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -758,15 +758,6 @@ static int at24_probe(struct i2c_client *client)
>         }
>         pm_runtime_enable(dev);
>
> -       at24->nvmem =3D devm_nvmem_register(dev, &nvmem_config);
> -       if (IS_ERR(at24->nvmem)) {
> -               pm_runtime_disable(dev);
> -               if (!pm_runtime_status_suspended(dev))
> -                       regulator_disable(at24->vcc_reg);
> -               return dev_err_probe(dev, PTR_ERR(at24->nvmem),
> -                                    "failed to register nvmem\n");
> -       }
> -
>         /*
>          * Perform a one-byte test read to verify that the chip is functi=
onal,
>          * unless powering on the device is to be avoided during probe (i=
e.
> @@ -782,6 +773,15 @@ static int at24_probe(struct i2c_client *client)
>                 }
>         }
>
> +       at24->nvmem =3D devm_nvmem_register(dev, &nvmem_config);
> +       if (IS_ERR(at24->nvmem)) {
> +               pm_runtime_disable(dev);
> +               if (!pm_runtime_status_suspended(dev))
> +                       regulator_disable(at24->vcc_reg);
> +               return dev_err_probe(dev, PTR_ERR(at24->nvmem),
> +                                    "failed to register nvmem\n");
> +       }
> +
>         /* If this a SPD EEPROM, probe for DDR3 thermal sensor */
>         if (cdata =3D=3D &at24_data_spd)
>                 at24_probe_temp_sensor(client);
> --
> 2.44.0.683.g7961c838ac-goog
>

Looks good, can you add a Fixes tag?

Thanks,
Bartosz