| Message ID | 20240306130042.26811-1-hyperlyzcs@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: misc: ljca: Fix double free in error handling path | expand |
Hi, On 3/6/24 14:00, Yongzhi Liu wrote: > When auxiliary_device_add() returns error and then calls > auxiliary_device_uninit(), callback function ljca_auxdev_release > calls kfree(auxdev->dev.platform_data) to free the parameter data > of the function ljca_new_client_device. The callers of > ljca_new_client_device shouldn't call kfree() again > in the error handling path to free the platform data. > > Fix this by cleaning up the redundant kfree(). If things fail in ljca_new_client_device() before auxiliary_device_init() gets called (or if that call fails) then auxiliary_device_uninit() will NOT be called leading to a memory-leak. So this patch is no good as is. To properly fix this you must make ljca_new_client_device() always take ownership of the passed in platform_data and make it also kfree() the passed in platform_data on errors which happen before auxiliary_device_init() succeeds. Regards, Hans > > Fixes: acd6199f195d ("usb: Add support for Intel LJCA device") > Signed-off-by: Yongzhi Liu <hyperlyzcs@gmail.com> > --- > drivers/usb/misc/usb-ljca.c | 18 +++--------------- > 1 file changed, 3 insertions(+), 15 deletions(-) > > diff --git a/drivers/usb/misc/usb-ljca.c b/drivers/usb/misc/usb-ljca.c > index 35770e608c64..be702364be08 100644 > --- a/drivers/usb/misc/usb-ljca.c > +++ b/drivers/usb/misc/usb-ljca.c > @@ -590,12 +590,8 @@ static int ljca_enumerate_gpio(struct ljca_adapter *adap) > valid_pin[i] = get_unaligned_le32(&desc->bank_desc[i].valid_pins); > bitmap_from_arr32(gpio_info->valid_pin_map, valid_pin, gpio_num); > > - ret = ljca_new_client_device(adap, LJCA_CLIENT_GPIO, 0, "ljca-gpio", > + return ljca_new_client_device(adap, LJCA_CLIENT_GPIO, 0, "ljca-gpio", > gpio_info, LJCA_GPIO_ACPI_ADR); > - if (ret) > - kfree(gpio_info); > - > - return ret; > } > > static int ljca_enumerate_i2c(struct ljca_adapter *adap) > @@ -626,13 +622,9 @@ static int ljca_enumerate_i2c(struct ljca_adapter *adap) > i2c_info->capacity = desc->info[i].capacity; > i2c_info->intr_pin = desc->info[i].intr_pin; > > - ret = ljca_new_client_device(adap, LJCA_CLIENT_I2C, i, > + return ljca_new_client_device(adap, LJCA_CLIENT_I2C, i, > "ljca-i2c", i2c_info, > LJCA_I2C1_ACPI_ADR + i); > - if (ret) { > - kfree(i2c_info); > - return ret; > - } > } > > return 0; > @@ -666,13 +658,9 @@ static int ljca_enumerate_spi(struct ljca_adapter *adap) > spi_info->id = desc->info[i].id; > spi_info->capacity = desc->info[i].capacity; > > - ret = ljca_new_client_device(adap, LJCA_CLIENT_SPI, i, > + return ljca_new_client_device(adap, LJCA_CLIENT_SPI, i, > "ljca-spi", spi_info, > LJCA_SPI1_ACPI_ADR + i); > - if (ret) { > - kfree(spi_info); > - return ret; > - } > } > > return 0;
diff --git a/drivers/usb/misc/usb-ljca.c b/drivers/usb/misc/usb-ljca.c index 35770e608c64..be702364be08 100644 --- a/drivers/usb/misc/usb-ljca.c +++ b/drivers/usb/misc/usb-ljca.c @@ -590,12 +590,8 @@ static int ljca_enumerate_gpio(struct ljca_adapter *adap) valid_pin[i] = get_unaligned_le32(&desc->bank_desc[i].valid_pins); bitmap_from_arr32(gpio_info->valid_pin_map, valid_pin, gpio_num); - ret = ljca_new_client_device(adap, LJCA_CLIENT_GPIO, 0, "ljca-gpio", + return ljca_new_client_device(adap, LJCA_CLIENT_GPIO, 0, "ljca-gpio", gpio_info, LJCA_GPIO_ACPI_ADR); - if (ret) - kfree(gpio_info); - - return ret; } static int ljca_enumerate_i2c(struct ljca_adapter *adap) @@ -626,13 +622,9 @@ static int ljca_enumerate_i2c(struct ljca_adapter *adap) i2c_info->capacity = desc->info[i].capacity; i2c_info->intr_pin = desc->info[i].intr_pin; - ret = ljca_new_client_device(adap, LJCA_CLIENT_I2C, i, + return ljca_new_client_device(adap, LJCA_CLIENT_I2C, i, "ljca-i2c", i2c_info, LJCA_I2C1_ACPI_ADR + i); - if (ret) { - kfree(i2c_info); - return ret; - } } return 0; @@ -666,13 +658,9 @@ static int ljca_enumerate_spi(struct ljca_adapter *adap) spi_info->id = desc->info[i].id; spi_info->capacity = desc->info[i].capacity; - ret = ljca_new_client_device(adap, LJCA_CLIENT_SPI, i, + return ljca_new_client_device(adap, LJCA_CLIENT_SPI, i, "ljca-spi", spi_info, LJCA_SPI1_ACPI_ADR + i); - if (ret) { - kfree(spi_info); - return ret; - } } return 0;
When auxiliary_device_add() returns error and then calls auxiliary_device_uninit(), callback function ljca_auxdev_release calls kfree(auxdev->dev.platform_data) to free the parameter data of the function ljca_new_client_device. The callers of ljca_new_client_device shouldn't call kfree() again in the error handling path to free the platform data. Fix this by cleaning up the redundant kfree(). Fixes: acd6199f195d ("usb: Add support for Intel LJCA device") Signed-off-by: Yongzhi Liu <hyperlyzcs@gmail.com> --- drivers/usb/misc/usb-ljca.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-)