Re: [PATCH v1] at24: fix memory corruption race condition

Bartosz Golaszewski
2024-04-17T23:59:59Z

On Thu, Apr 18, 2024 at 1:07 AM Daniel Okazaki <dtokazaki@google.com> 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 = 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 functional,
> * 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 = 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 == &at24_data_spd)
> at24_probe_temp_sensor(client);
> --
> 2.44.0.683.g7961c838ac-goog
>

Looks good, can you add a Fixes tag?

Thanks,
Bartosz