On Sun, Nov 16, 2025 at 09:49:48AM +0800, Ma Ke wrote:
lpc32xx_udc_probe() acquires an i2c_client reference through isp1301_get_client() but fails to release it in both error handling paths and the normal removal path. This could result in a reference count leak for the I2C device, preventing proper cleanup and potentially leading to resource exhaustion. Add put_device() to release the reference in the probe failure path and in the remove function.
Calling path: isp1301_get_client() -> of_find_i2c_device_by_node() -> i2c_find_device_by_fwnode(). As comments of i2c_find_device_by_fwnode() says, 'The user must call put_device(&client->dev) once done with the i2c client.'
Found by code review.
Cc: stable@vger.kernel.org Fixes: 24a28e428351 ("USB: gadget driver for LPC32xx") Signed-off-by: Ma Ke make24@iscas.ac.cn
drivers/usb/gadget/udc/lpc32xx_udc.c | 35 +++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c index 1a7d3c4f652f..b6fddfff712d 100644 --- a/drivers/usb/gadget/udc/lpc32xx_udc.c +++ b/drivers/usb/gadget/udc/lpc32xx_udc.c @@ -2986,6 +2986,7 @@ static int lpc32xx_udc_probe(struct platform_device *pdev) int retval, i; dma_addr_t dma_handle; struct device_node *isp1301_node;
- bool isp1301_acquired = false;
This bool should not be needed, you "know" if you have acquired this or not by virtue of being later in the function call.
udc = devm_kmemdup(dev, &controller_template, sizeof(*udc), GFP_KERNEL); if (!udc) @@ -3013,6 +3014,7 @@ static int lpc32xx_udc_probe(struct platform_device *pdev) if (!udc->isp1301_i2c_client) { return -EPROBE_DEFER; }
- isp1301_acquired = true;
dev_info(udc->dev, "ISP1301 I2C device at address 0x%x\n", udc->isp1301_i2c_client->addr); @@ -3020,7 +3022,7 @@ static int lpc32xx_udc_probe(struct platform_device *pdev) pdev->dev.dma_mask = &lpc32xx_usbd_dmamask; retval = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32)); if (retval)
return retval;
goto i2c_fail;udc->board = &lpc32xx_usbddata; @@ -3038,28 +3040,32 @@ static int lpc32xx_udc_probe(struct platform_device *pdev) /* Get IRQs */ for (i = 0; i < 4; i++) { udc->udp_irq[i] = platform_get_irq(pdev, i);
if (udc->udp_irq[i] < 0)return udc->udp_irq[i];
if (udc->udp_irq[i] < 0) {retval = udc->udp_irq[i];goto i2c_fail;}udc->udp_baseaddr = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(udc->udp_baseaddr)) { dev_err(udc->dev, "IO map failure\n");
return PTR_ERR(udc->udp_baseaddr);
retval = PTR_ERR(udc->udp_baseaddr);goto i2c_fail;/* Get USB device clock */ udc->usb_slv_clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(udc->usb_slv_clk)) { dev_err(udc->dev, "failed to acquire USB device clock\n");
return PTR_ERR(udc->usb_slv_clk);
retval = PTR_ERR(udc->usb_slv_clk);goto i2c_fail;/* Enable USB device clock */ retval = clk_prepare_enable(udc->usb_slv_clk); if (retval < 0) { dev_err(udc->dev, "failed to start USB device clock\n");
return retval;
goto i2c_fail;/* Setup deferred workqueue data */ @@ -3161,6 +3167,8 @@ static int lpc32xx_udc_probe(struct platform_device *pdev) dma_free_coherent(&pdev->dev, UDCA_BUFF_SIZE, udc->udca_v_base, udc->udca_p_base); i2c_fail:
- if (isp1301_acquired && udc->isp1301_i2c_client)
put_device(&udc->isp1301_i2c_client->dev);@@ -3170,6 +3178,18 @@ static int lpc32xx_udc_probe(struct platform_device *pdev) static void lpc32xx_udc_remove(struct platform_device *pdev) { struct lpc32xx_udc *udc = platform_get_drvdata(pdev);
- struct device *dev = &pdev->dev;
- struct device_node *isp1301_node;
- bool isp1301_acquired = false;
This bool isn't needed either, just trigger off of isp1301_node.
But really:
- /* Check if we acquired isp1301 via device tree */
- if (dev->of_node) {
isp1301_node = of_parse_phandle(dev->of_node, "transceiver", 0);
Shouldn't this node be saved in the device structure instead? That's the "correct" solution here.
thanks,
greg k-h