mfd: Assorted resource lifetime, teardown, and error-handling fixes
Subsystems (16)
Lee Jones
maintainerv2 cover mfd: Assorted resource lifetime, teardown, and error-handling fixes
These issues were reported by Sashiko.
This series addresses a collection of resource lifetime, teardown ordering, error-handling, and bounds-checking bugs across several MFD drivers.
v1 => v2:
- Collected Acked-by, Reviewed-by, and Tested-by tags from the list
- Dropped "mfd: cgbc: Zero-initialise version buffer in cgbc_get_version()" in favour of Thomas Richard's upcoming central cgbc_command() validation series
- Restored Fixes: tags (without Cc: [email protected]) on the four non-Stable MFD cell stack-pointer clean-ups (ls2kbmc, bd96801, ti_am335x_tscadc, and db8500-prcmu)
Lee Jones (27):
mfd: nct6694: Allocate USB DMA buffers with kzalloc_obj()
mfd: ls2kbmc: Unregister GPIO GSI via devm_add_action_or_reset()
mfd: ls2kbmc: Cancel reset work on teardown via devm_work_autocancel()
mfd: ls2kbmc: Allocate simplefb platform_data on the heap
mfd: bd96801: Allocate watchdog IRQ resource on the heap
mfd: ti_am335x_tscadc: Retrieve parent driver data via
dev_get_drvdata()
mfd: db8500-prcmu: Make AB8500 cell and IRQ resource static
mfd: nct6694: Do not resubmit interrupt URB on endpoint stall (-EPIPE)
mfd: ioc3: Request parent IRQ via request_irq() and free before domain
removal
mfd: intel-lpss: Remove registered MFD child devices on probe failure
mfd: max8907: Manage power-off handler via
devm_register_sys_off_handler()
mfd: twl-core: Clear pm_power_off callback in twl_remove()
mfd: twl-core: Clear twl_priv pointer on teardown and probe failure
mfd: loongson-se: Check return value of dma_set_mask_and_coherent()
mfd: loongson-se: Zero-initialise controller command structures
mfd: loongson-se: Reinitialise completions and wait uninterruptibly
for commands
mfd: loongson-se: Zero-allocate driver data and initialise engine
completions in probe
mfd: loongson-se: Prevent out-of-bounds engine access in
se_irq_handler()
mfd: loongson-se: Stop controller and mask interrupts on teardown and
shutdown
mfd: loongson-se: Validate engine ID, DMA buffer size, and IRQ
registration
mfd: abx500-core: Unregister device ops on devm release
mfd: ab8500-core: Register sysfs attribute groups via
devm_device_add_group()
mfd: tps65217: Manage IRQ domain teardown via devres
mfd: mt6397: Manage IRQ domain lifecycle with devres
mfd: mt6397: Unregister PM notifier on probe failure and unbind
i2c: cgbc: Propagate hardware errors when polling read status
mfd: ti_am335x_tscadc: Remove redundant ti_tscadc_dev_get() helper
drivers/i2c/busses/i2c-cgbc.c | 27 ++++-- drivers/iio/adc/ti_am335x_adc.c | 2 +- drivers/input/touchscreen/ti_am335x_tsc.c | 2 +- drivers/mfd/ab8500-core.c | 11 +-- drivers/mfd/abx500-core.c | 29 +++--- drivers/mfd/db8500-prcmu.c | 8 +- drivers/mfd/intel-lpss.c | 1 + drivers/mfd/ioc3.c | 32 ++++--- drivers/mfd/loongson-se.c | 110 +++++++++++++++++----- drivers/mfd/ls2k-bmc-core.c | 27 ++++-- drivers/mfd/max8907.c | 28 ++++-- drivers/mfd/mt6358-irq.c | 17 +++- drivers/mfd/mt6397-core.c | 4 +- drivers/mfd/mt6397-irq.c | 31 +++++- drivers/mfd/nct6694.c | 23 +++-- drivers/mfd/rohm-bd96801.c | 11 ++- drivers/mfd/ti_am335x_tscadc.c | 7 +- drivers/mfd/tps65217.c | 40 ++++---- drivers/mfd/twl-core.c | 45 +++++---- include/linux/mfd/loongson-se.h | 8 +- include/linux/mfd/ti_am335x_tscadc.h | 7 -- 21 files changed, 326 insertions(+), 144 deletions(-)
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 1/27 mfd: nct6694: Allocate USB DMA buffers with kzalloc_obj()
nct6694_usb_probe() allocates the USB transfer buffers usb_msg and int_buffer using devm_kzalloc(). Because devm_kzalloc() prepends internal devres metadata to the allocation, the returned buffer pointers are not guaranteed to be ARCH_DMA_MINALIGN cacheline-aligned. Mapping these unaligned buffers for USB bulk and interrupt DMA transfers risks cacheline sharing and memory corruption on non-cache-coherent architectures.
Allocate usb_msg and int_buffer using kzalloc_obj() to guarantee DMA-safe alignment, and free them on probe failure and in nct6694_usb_disconnect().
--- drivers/mfd/nct6694.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)
drivers/mfd/nct6694.c +16 -6
@@ -284,17 +284,21 @@ static int nct6694_usb_probe(struct usb_interface *iface,if (!nct6694)return -ENOMEM ; - nct6694->usb_msg =devm_kzalloc (dev,sizeof (union nct6694_usb_msg ),GFP_KERNEL ); + nct6694->usb_msg =kzalloc_obj (*nct6694->usb_msg);if (!nct6694->usb_msg)return -ENOMEM ; - nct6694->int_buffer =devm_kzalloc (dev,sizeof (*nct6694->int_buffer),GFP_KERNEL ); -if (!nct6694->int_buffer) -return -ENOMEM ; + nct6694->int_buffer =kzalloc_obj (*nct6694->int_buffer); +if (!nct6694->int_buffer) { + ret = -ENOMEM ; + goto err_usb_msg; + } nct6694->int_in_urb =usb_alloc_urb (0 ,GFP_KERNEL ); -if (!nct6694->int_in_urb) -return -ENOMEM ; +if (!nct6694->int_in_urb) { + ret = -ENOMEM ; + goto err_int_buffer; + } nct6694->domain = irq_domain_create_simple(NULL, NCT6694_NR_IRQS, 0, &nct6694_irq_domain_ops,
@@ -352,6 +356,10 @@ static int nct6694_usb_probe(struct usb_interface *iface,irq_domain_remove (nct6694->domain); err_urb:usb_free_urb (nct6694->int_in_urb); +err_int_buffer: +kfree (nct6694->int_buffer); +err_usb_msg: +kfree (nct6694->usb_msg);return ret; }
@@ -367,6 +375,8 @@ static void nct6694_usb_disconnect(struct usb_interface *iface)ida_destroy (&nct6694->gpio_ida);irq_domain_remove (nct6694->domain);usb_free_urb (nct6694->int_in_urb); +kfree (nct6694->int_buffer); +kfree (nct6694->usb_msg); }static const struct usb_device_id nct6694_ids[] = {
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 2/27 mfd: ls2kbmc: Unregister GPIO GSI via devm_add_action_or_reset()
In reply to Lee Jonesls2k_bmc_init() unconditionally calls acpi_unregister_gsi() immediately after requesting the GPIO interrupt via devm_request_irq(). This prematurely disposes of the active IRQ mapping while the interrupt handler remains registered, breaking the GPIO watchdog interrupt and causing a use-after-free when devres later frees the disposed IRQ descriptor on probe failure or device unbind.
Register a devm_add_action_or_reset() callback before devm_request_irq() so that acpi_unregister_gsi() is invoked only after devres has freed the interrupt.
--- drivers/mfd/ls2k-bmc-core.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/mfd/ls2k-bmc-core.c +9 -1
@@ -363,6 +363,11 @@ static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_ddata *pci_read_config_dword (pdev,PCI_INTERRUPT_LINE , &ddata->bmc_pci_data.interrupt_line); } +static void ls2k_bmc_unregister_gsi (void *data) +{ +acpi_unregister_gsi (LS2K_BMC_RESET_GPIO_GSI ); +} +static int ls2k_bmc_init (struct ls2k_bmc_ddata *ddata) {struct pci_dev *pdev =to_pci_dev (ddata->dev);
@@ -412,12 +417,15 @@ static int ls2k_bmc_init(struct ls2k_bmc_ddata *ddata)if (gpio_irq <0 )return gpio_irq; + ret =devm_add_action_or_reset (ddata->dev, ls2k_bmc_unregister_gsi,NULL ); +if (ret) +return ret; + ret =devm_request_irq (ddata->dev, gpio_irq, ls2k_bmc_interrupt,IRQF_SHARED |IRQF_TRIGGER_FALLING ,"ls2kbmc gpio" , ddata);if (ret)dev_err (ddata->dev,"Failed to request LS2KBMC GPIO IRQ %d.\n" , gpio_irq); -acpi_unregister_gsi (LS2K_BMC_RESET_GPIO_GSI );return ret; }
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 3/27 mfd: ls2kbmc: Cancel reset work on teardown via devm_work_autocancel()
In reply to Lee Jonesls2k_bmc_init() initialises bmc_reset_work with INIT_WORK() and immediately requests the PCIe and GPIO interrupts via devm_request_irq(). If an interrupt fires and schedules bmc_reset_work before a subsequent step in ls2k_bmc_probe() fails, or when the device is unbound, devres frees ddata whilst bmc_reset_work remains queued, leading to a use-after-free in ls2k_bmc_events_fn().
Initialise bmc_reset_work with devm_work_autocancel() before requesting the interrupts so that devres cancels any pending work after the interrupts are freed and before ddata is released.
--- drivers/mfd/ls2k-bmc-core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/mfd/ls2k-bmc-core.c +4 -1
@@ -12,6 +12,7 @@#include <linux/aperture.h> #include <linux/bitfield.h> #include <linux/delay.h> +#include <linux/devm-helpers.h> #include <linux/errno.h> #include <linux/init.h> #include <linux/iopoll.h>
@@ -376,7 +377,9 @@ static int ls2k_bmc_init(struct ls2k_bmc_ddata *ddata)ls2k_bmc_save_pci_data (pdev, ddata); -INIT_WORK (&ddata->bmc_reset_work, ls2k_bmc_events_fn); + ret =devm_work_autocancel (ddata->dev, &ddata->bmc_reset_work, ls2k_bmc_events_fn); +if (ret) +return ret; ret =devm_request_irq (&pdev->dev, pdev->irq, ls2k_bmc_interrupt,IRQF_SHARED |IRQF_TRIGGER_FALLING ,"ls2kbmc pcie" , ddata);
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 4/27 mfd: ls2kbmc: Allocate simplefb platform_data on the heap
In reply to Lee Jonesls2k_bmc_probe() declares struct simplefb_platform_data on the stack and assigns its address to the display MFD cell. Although mfd_add_device() deep-copies platform_data via platform_device_add_data() before ls2k_bmc_probe() returns, the shallow mfd_cell copy attached to the child platform device retains a stale pointer to the expired stack frame.
Allocate the simplefb_platform_data structure on the heap via devm_kzalloc() prior to devm_mfd_add_devices() so the pointer remains valid for the lifetime of the child platform device.
--- drivers/mfd/ls2k-bmc-core.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/mfd/ls2k-bmc-core.c +8 -4
@@ -470,7 +470,7 @@ static int ls2k_bmc_parse_mode(struct pci_dev *pdev, struct simplefb_platform_dastatic int ls2k_bmc_probe (struct pci_dev *dev,const struct pci_device_id *id) { -struct simplefb_platform_data pd; +struct simplefb_platform_data *pd;struct ls2k_bmc_ddata *ddata;resource_size_t base;int ret;
@@ -489,12 +489,16 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)if (ret)return ret; - ret =ls2k_bmc_parse_mode (dev, &pd); + pd =devm_kzalloc (&dev->dev,sizeof (*pd),GFP_KERNEL ); +if (!pd) +return -ENOMEM ; + + ret =ls2k_bmc_parse_mode (dev, pd);if (ret)return ret; - ls2k_bmc_cells[LS2K_BMC_DISPLAY ].platform_data = &pd; - ls2k_bmc_cells[LS2K_BMC_DISPLAY ].pdata_size =sizeof (pd); + ls2k_bmc_cells[LS2K_BMC_DISPLAY ].platform_data = pd; + ls2k_bmc_cells[LS2K_BMC_DISPLAY ].pdata_size =sizeof (*pd); base =pci_resource_start (dev,0 ) +LS2K_DISPLAY_RES_START ;/* Remove conflicting efifb device */
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 5/27 mfd: bd96801: Allocate watchdog IRQ resource on the heap
In reply to Lee Jonesbd96801_i2c_probe() declares struct resource wdg_irq on the stack and assigns its address to ddata->cells[WDG_CELL].resources. Although mfd_add_device() deep-copies the resource table via platform_device_add_resources() before bd96801_i2c_probe() returns, the shared cell template and the shallow MFD cell copy attached to the child platform device retain a stale pointer to the expired stack frame.
Allocate wdg_irq on the heap via devm_kzalloc() alongside regulator_res so the stored pointer remains valid for the lifetime of the device.
--- drivers/mfd/rohm-bd96801.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
drivers/mfd/rohm-bd96801.c +7 -4
@@ -642,8 +642,7 @@ static int bd96801_i2c_probe(struct i2c_client *i2c)struct irq_domain *intb_domain, *errb_domain;const struct bd968xx *ddata;const struct fwnode_handle *fwnode; -struct resource *regulator_res; -struct resource wdg_irq; +struct resource *regulator_res, *wdg_irq;struct regmap *regmap;int intb_irq, errb_irq, num_errb =0 ;int num_regu_irqs, wdg_irq_no;
@@ -692,6 +691,10 @@ static int bd96801_i2c_probe(struct i2c_client *i2c)if (!regulator_res)return -ENOMEM ; + wdg_irq =devm_kzalloc (&i2c->dev,sizeof (*wdg_irq),GFP_KERNEL ); +if (!wdg_irq) +return -ENOMEM ; + regmap =devm_regmap_init_i2c (i2c, ddata->regmap_config);if (IS_ERR (regmap)) return dev_err_probe(&i2c->dev, PTR_ERR(regmap),
@@ -723,9 +726,9 @@ static int bd96801_i2c_probe(struct i2c_client *i2c) } wdg_irq_no =irq_create_mapping (intb_domain,BD96801_WDT_ERR_STAT ); - wdg_irq =DEFINE_RES_IRQ_NAMED (wdg_irq_no,"bd96801-wdg" ); + *wdg_irq =DEFINE_RES_IRQ_NAMED (wdg_irq_no,"bd96801-wdg" ); - ddata->cells[WDG_CELL ].resources = &wdg_irq; + ddata->cells[WDG_CELL ].resources = wdg_irq; ddata->cells[WDG_CELL ].num_resources =1 ;if (!num_errb)
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 6/27 mfd: ti_am335x_tscadc: Retrieve parent driver data via dev_get_drvdata()
In reply to Lee Jonesti_tscadc_probe() stores the address of the local pointer variable tscadc (&tscadc) into cell->platform_data so child devices can retrieve the parent struct ti_tscadc_dev pointer via ti_tscadc_dev_get(). Although mfd_add_device() deep-copies platform_data before ti_tscadc_probe() returns, both tscadc->cells[].platform_data and the shallow MFD cell copy attached to the child platform devices retain a stale pointer to the expired stack frame.
Set the parent platform driver data before registering the MFD child devices, update ti_tscadc_dev_get() to retrieve the parent driver data via dev_get_drvdata(p->dev.parent), and drop the redundant cell platform_data assignment.
--- drivers/mfd/ti_am335x_tscadc.c | 7 ++----- include/linux/mfd/ti_am335x_tscadc.h | 4 +--- 2 files changed, 3 insertions(+), 8 deletions(-)
drivers/mfd/ti_am335x_tscadc.c +2 -5
@@ -268,8 +268,6 @@ static int ti_tscadc_probe(struct platform_device *pdev) cell = &tscadc->cells[cell_idx++]; cell->name = tscadc->data->secondary_feature_name; cell->of_compatible = tscadc->data->secondary_feature_compatible; - cell->platform_data = &tscadc; - cell->pdata_size =sizeof (tscadc); }/* ADC Cell */
@@ -277,16 +275,15 @@ static int ti_tscadc_probe(struct platform_device *pdev) cell = &tscadc->cells[cell_idx++]; cell->name = tscadc->data->adc_feature_name; cell->of_compatible = tscadc->data->adc_feature_compatible; - cell->platform_data = &tscadc; - cell->pdata_size =sizeof (tscadc); } +platform_set_drvdata (pdev, tscadc); + err =mfd_add_devices (&pdev->dev,PLATFORM_DEVID_AUTO , tscadc->cells, cell_idx,NULL ,0 ,NULL );if (err <0 ) goto err_disable_clk; -platform_set_drvdata (pdev, tscadc);return 0 ; err_disable_clk:
include/linux/mfd/ti_am335x_tscadc.h +1 -3
@@ -177,9 +177,7 @@ struct ti_tscadc_dev {static inline struct ti_tscadc_dev *ti_tscadc_dev_get (struct platform_device *p) { -struct ti_tscadc_dev **tscadc_dev = p->dev.platform_data; - -return *tscadc_dev; +return dev_get_drvdata (p->dev.parent); }static inline bool ti_adc_with_touchscreen (struct ti_tscadc_dev *tscadc)
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 7/27 mfd: db8500-prcmu: Make AB8500 cell and IRQ resource static
In reply to Lee Jonesdb8500_prcmu_register_ab8500() declares struct resource ab850x_resource and the mfd_cell descriptors on the stack and assigns &ab850x_resource to ab850x_cell->resources. Although mfd_add_device() deep-copies the resource table via platform_device_add_resources() before db8500_prcmu_register_ab8500() returns, the shallow MFD cell copy attached to the child platform device retains a stale pointer to the expired stack frame.
Mark ab850x_resource as static and the ab8500_cell and ab8505_cell descriptors as static const so the resource pointer remains valid for the lifetime of the device.
--- drivers/mfd/db8500-prcmu.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/mfd/db8500-prcmu.c +4 -4
@@ -2953,16 +2953,15 @@ static const struct mfd_cell db8500_prcmu_devs[] = {static int db8500_prcmu_register_ab8500 (struct device *parent) { -struct device_node *np; -struct resource ab850x_resource; -const struct mfd_cell ab8500_cell = { +static struct resource ab850x_resource; +static const struct mfd_cell ab8500_cell = { .name ="ab8500-core" , .of_compatible ="stericsson,ab8500" , .id =AB8500_VERSION_AB8500 , .resources = &ab850x_resource, .num_resources =1 , }; -const struct mfd_cell ab8505_cell = { +static const struct mfd_cell ab8505_cell = { .name ="ab8505-core" , .of_compatible ="stericsson,ab8505" , .id =AB8500_VERSION_AB8505 ,
@@ -2970,6 +2969,7 @@ static int db8500_prcmu_register_ab8500(struct device *parent) .num_resources = 1, };const struct mfd_cell *ab850x_cell; +struct device_node *np;if (!parent->of_node)return -ENODEV ;
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 8/27 mfd: nct6694: Do not resubmit interrupt URB on endpoint stall (-EPIPE)
In reply to Lee JonesIf the USB interrupt IN endpoint stalls (-EPIPE), usb_int_callback() falls through to the default case and immediately resubmits the URB from atomic completion context. Because a stalled endpoint completes newly submitted URBs immediately with -EPIPE, this triggers a tight infinite resubmission loop.
Treat -EPIPE as a terminal URB status in usb_int_callback() alongside the existing teardown status codes, matching other USB MFD drivers.
--- drivers/mfd/nct6694.c | 1 + 1 file changed, 1 insertion(+)
drivers/mfd/nct6694.c +1 -0
@@ -201,6 +201,7 @@ static void usb_int_callback(struct urb *urb)break ;case -ECONNRESET :case -ENOENT : +case -EPIPE :case -ESHUTDOWN :return ;default :
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 9/27 mfd: ioc3: Request parent IRQ via request_irq() and free before domain removal
In reply to Lee Jonesioc3_irq_domain_setup() registers ioc3_irq_handler() as a chained interrupt handler using irq_set_chained_handler_and_data(), whilst ioc3_mfd_remove() and the ioc3_mfd_probe() error path attempt to tear it down with free_irq(ipd->domain_irq, (void *)ipd) after removing the IRQ domain. Because chained handlers bypass the standard irqaction list, free_irq() warns and fails to detach ioc3_irq_handler(), leaving a dangling handler referencing the freed IRQ domain and driver private data.
Register ioc3_irq_handler() with request_irq() to match free_irq(), dispatch child interrupts via generic_handle_domain_irq_safe(), and free the IRQ prior to removing the IRQ domain on teardown.
--- drivers/mfd/ioc3.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-)
drivers/mfd/ioc3.c +20 -12
@@ -99,10 +99,9 @@ static const struct irq_domain_ops ioc3_irq_domain_ops = { .unmap = ioc3_irq_domain_unmap, }; -static void ioc3_irq_handler (struct irq_desc *desc) +static irqreturn_t ioc3_irq_handler (int irq,void *data) { -struct irq_domain *domain =irq_desc_get_handler_data (desc); -struct ioc3_priv_data *ipd = domain->host_data; +struct ioc3_priv_data *ipd = data;struct ioc3 __iomem *regs = ipd->regs;u32 pending, mask;
@@ -110,10 +109,12 @@ static void ioc3_irq_handler(struct irq_desc *desc) mask =readl (®s->sio_ies); pending &= mask;/* Mask off not enabled interrupts */ -if (pending) -generic_handle_domain_irq (domain,__ffs (pending)); -else -spurious_interrupt (); +if (pending) { +generic_handle_domain_irq_safe (ipd->domain,__ffs (pending)); +return IRQ_HANDLED ; + } + +return IRQ_NONE ; }/*
@@ -133,6 +134,7 @@ static int ioc3_irq_domain_setup(struct ioc3_priv_data *ipd, int irq) {struct irq_domain *domain;struct fwnode_handle *fn; +int ret = -ENOMEM ; fn =irq_domain_alloc_named_fwnode ("IOC3" );if (!fn)
@@ -146,13 +148,19 @@ static int ioc3_irq_domain_setup(struct ioc3_priv_data *ipd, int irq) ipd->domain = domain; -irq_set_chained_handler_and_data (irq, ioc3_irq_handler, domain); + ret =request_irq (irq, ioc3_irq_handler,IRQF_SHARED ,"IOC3" , ipd); +if (ret) { +irq_domain_remove (domain); +irq_domain_free_fwnode (fn); + ipd->domain =NULL ; + goto err; + } ipd->domain_irq = irq;return 0 ; err:dev_err (&ipd->pdev->dev,"irq domain setup failed\n" ); -return -ENOMEM ; +return ret; }static const struct resource ioc3_uarta_resources[] = {
@@ -615,9 +623,9 @@ static int ioc3_mfd_probe(struct pci_dev *pdev,if (ipd->domain) {struct fwnode_handle *fn = ipd->domain->fwnode; +free_irq (ipd->domain_irq, (void *)ipd);irq_domain_remove (ipd->domain);irq_domain_free_fwnode (fn); -free_irq (ipd->domain_irq, (void *)ipd); }pci_iounmap (pdev, regs); goto out_disable_device;
@@ -645,9 +653,9 @@ static void ioc3_mfd_remove(struct pci_dev *pdev)if (ipd->domain) {struct fwnode_handle *fn = ipd->domain->fwnode; -irq_domain_remove (ipd->domain); -irq_domain_free_fwnode (fn);free_irq (ipd->domain_irq, (void *)ipd); +irq_domain_remove (ipd->domain); +irq_domain_free_fwnode (fn); }pci_iounmap (pdev, ipd->regs);pci_disable_device (pdev);
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 10/27 mfd: intel-lpss: Remove registered MFD child devices on probe failure
In reply to Lee JonesWhen intel_lpss_has_idma() is true, intel_lpss_probe() registers the iDMA64 child device via mfd_add_devices() before registering the host controller cell. If the second mfd_add_devices() call fails, the err_remove_ltr error path returns without calling mfd_remove_devices(), leaving the iDMA64 child device registered after the parent's device-managed resources are freed.
Call mfd_remove_devices() on the probe error path to clean up any previously registered MFD child devices, matching intel_lpss_remove().
--- drivers/mfd/intel-lpss.c | 1 + 1 file changed, 1 insertion(+)
drivers/mfd/intel-lpss.c +1 -0
@@ -455,6 +455,7 @@ int intel_lpss_probe(struct device *dev,return 0 ; err_remove_ltr: +mfd_remove_devices (dev);intel_lpss_debugfs_remove (lpss);intel_lpss_ltr_hide (lpss);intel_lpss_unregister_clock (lpss);
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 11/27 mfd: max8907: Manage power-off handler via devm_register_sys_off_handler()
In reply to Lee JonesWhen the MAX8907 device is configured as the system power controller, max8907_i2c_probe() assigns the devres-managed device context to max8907_pm_off and registers max8907_power_off() as the global pm_power_off handler. However, max8907_i2c_remove() does not clear these pointers upon device unbind, leaving pm_power_off and max8907_pm_off dangling after devres frees the driver state.
Convert max8907_power_off() to a sys_off_handler callback registered via devm_register_sys_off_handler() so the handler is automatically unregistered on device unbind and the global max8907_pm_off pointer can be dropped.
--- drivers/mfd/max8907.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-)
drivers/mfd/max8907.c +21 -7
@@ -15,6 +15,7 @@#include <linux/mfd/max8907.h> #include <linux/module.h> #include <linux/of.h> +#include <linux/reboot.h> #include <linux/regmap.h> #include <linux/slab.h>
@@ -173,11 +174,14 @@ static const struct regmap_irq_chip max8907_rtc_irq_chip = { .num_irqs = ARRAY_SIZE(max8907_rtc_irqs), }; -static struct max8907 *max8907_pm_off; -static void max8907_power_off (void ) +static int max8907_power_off (struct sys_off_data *data) { -regmap_update_bits (max8907_pm_off->regmap_gen,MAX8907_REG_RESET_CNFG , -MAX8907_MASK_POWER_OFF ,MAX8907_MASK_POWER_OFF ); +struct max8907 *max8907 = data->cb_data; + +regmap_update_bits (max8907->regmap_gen,MAX8907_REG_RESET_CNFG , +MAX8907_MASK_POWER_OFF ,MAX8907_MASK_POWER_OFF ); + +return NOTIFY_DONE ; }static int max8907_i2c_probe (struct i2c_client *i2c)
@@ -256,13 +260,23 @@ static int max8907_i2c_probe(struct i2c_client *i2c) goto err_add_devices; } -if (pm_off && !pm_power_off) { - max8907_pm_off = max8907; - pm_power_off = max8907_power_off; +if (pm_off) { + ret =devm_register_sys_off_handler (&i2c->dev, +SYS_OFF_MODE_POWER_OFF , +SYS_OFF_PRIO_DEFAULT , + max8907_power_off, max8907); +if (ret) { +dev_err (&i2c->dev, +"failed to register sys-off handler: %d\n" , + ret); + goto err_sys_off; + } }return 0 ; +err_sys_off: +mfd_remove_devices (max8907->dev); err_add_devices:regmap_del_irq_chip (max8907->i2c_gen->irq, max8907->irqc_rtc); err_irqc_rtc:
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 12/27 mfd: twl-core: Clear pm_power_off callback in twl_remove()
In reply to Lee JonesWhen a TWL6030-class device is configured as the system power controller, twl_probe() registers twl6030_power_off() as the global pm_power_off callback. However, twl_remove() does not clear pm_power_off when tearing down the device on late probe failure or driver unbind, leaving a dangling callback that dereferences the freed twl_priv context during system shutdown.
Reset pm_power_off to NULL in twl_remove() when it points to twl6030_power_off().
--- drivers/mfd/twl-core.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-)
drivers/mfd/twl-core.c +16 -13
@@ -672,10 +672,26 @@ static void clocks_init(struct device *dev)/*----------------------------------------------------------------------*/ +static void twl6030_power_off (void ) +{ +int err; +u8 val; + + err =twl_i2c_read_u8 (TWL_MODULE_PM_MASTER , &val,TWL6030_PHOENIX_DEV_ON ); +if (err) +return ; + + val |=TWL6030_APP_DEVOFF |TWL6030_CON_DEVOFF |TWL6030_MOD_DEVOFF ; +twl_i2c_write_u8 (TWL_MODULE_PM_MASTER , val,TWL6030_PHOENIX_DEV_ON ); +} +static void twl_remove (struct i2c_client *client) {unsigned i, num_slaves; +if (pm_power_off == twl6030_power_off) + pm_power_off =NULL ; +if (twl_class_is_4030 ())twl4030_exit_irq ();else
@@ -692,19 +708,6 @@ static void twl_remove(struct i2c_client *client) twl_priv->ready = false; } -static void twl6030_power_off (void ) -{ -int err; -u8 val; - - err =twl_i2c_read_u8 (TWL_MODULE_PM_MASTER , &val,TWL6030_PHOENIX_DEV_ON ); -if (err) -return ; - - val |=TWL6030_APP_DEVOFF |TWL6030_CON_DEVOFF |TWL6030_MOD_DEVOFF ; -twl_i2c_write_u8 (TWL_MODULE_PM_MASTER , val,TWL6030_PHOENIX_DEV_ON ); -} -static struct of_dev_auxdata twl_auxdata_lookup[] = {OF_DEV_AUXDATA ("ti,twl4030-gpio" ,0 ,"twl4030-gpio" ,NULL ),
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 13/27 mfd: twl-core: Clear twl_priv pointer on teardown and probe failure
In reply to Lee Jonestwl_probe() allocates the file-scope twl_priv state with devm_kzalloc() and checks for a non-NULL twl_priv at entry to reject multiple device instances. However, twl_remove() and the early error paths in twl_probe() never reset twl_priv to NULL, a failure when adding MFD child devices bypasses twl_remove() by jumping to free instead of fail, and twl_remove() tears down the IRQ domain and dummy I2C clients before child devices are unregistered. After devres frees the underlying memory on probe failure or unbind, twl_priv remains a dangling pointer, causing subsequent exported API calls to dereference freed memory and blocking future probe attempts with -EBUSY.
Unregister DT-populated and MFD child devices at the start of twl_remove(), ensure mfd_add_devices() failures unwind via fail, and reset twl_priv to NULL in twl_remove() and on probe error exit.
--- drivers/mfd/twl-core.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
drivers/mfd/twl-core.c +12 -4
@@ -689,6 +689,11 @@ static void twl_remove(struct i2c_client *client) {unsigned i, num_slaves; +of_platform_depopulate (&client->dev); + +if (twl_class_is_6030 ()) +mfd_remove_devices (&client->dev); +if (pm_power_off == twl6030_power_off) pm_power_off =NULL ;
@@ -706,6 +711,7 @@ static void twl_remove(struct i2c_client *client) twl->client =NULL ; } twl_priv->ready = false; + twl_priv =NULL ; }
@@ -880,10 +886,10 @@ twl_probe(struct i2c_client *client) num_cells =ARRAY_SIZE (twl6030_cells); } - status =devm_mfd_add_devices (&client->dev,PLATFORM_DEVID_NONE , - cells, num_cells,NULL ,0 ,NULL ); + status =mfd_add_devices (&client->dev,PLATFORM_DEVID_NONE , + cells, num_cells,NULL ,0 ,NULL );if (status <0 ) - goto free; + goto fail;if (of_device_is_system_power_controller (node)) {if (!pm_power_off)
@@ -900,8 +906,10 @@ twl_probe(struct i2c_client *client)if (status <0 )twl_remove (client); free: -if (status <0 ) +if (status <0 ) {platform_device_unregister (pdev); + twl_priv =NULL ; + }return status; }
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Andreas Kemnade
maintainerRe: [PATCH v2 13/27] mfd: twl-core: Clear twl_priv pointer on teardown and probe failure
Quoted 11 lines · On Tue, 27 Oct 2026 09:30:42 +0000 Lee Jones <[email protected]> wrote:
twl_probe()allocates the file-scopetwl_privstate withdevm_kzalloc()and checks for a non-NULLtwl_privat entry to reject multiple device instances. However,twl_remove()and the early error paths intwl_probe()never resettwl_privto NULL, a failure when adding MFD child devices bypassestwl_remove()by jumping to free instead of fail, andtwl_remove()tears down the IRQ domain and dummy I2C clients before child devices are unregistered. After devres frees the underlying memory on probe failure or unbind,twl_privremains a dangling pointer, causing subsequent exported API calls to dereference freed memory and blocking future probe attempts with-EBUSY.
Quoted 8 lines
Unregister DT-populated and MFD child devices at the start of
twl_remove(), ensuremfd_add_devices()failures unwind via fail, and resettwl_privto NULL intwl_remove()and on probe error exit.Fixes 80a97ccd33be ("mfd: twl-core: Collect global variables behind one private structure (global)")Signed-off-by Lee Jones <[email protected]>
Hmm, couldn't we use devm_add_action_or_reset() for resetting the twl_priv pointer? Feels more clean for me.
Regards,
Andreas
Lee Jones
maintainerRe: [PATCH v2 13/27] mfd: twl-core: Clear twl_priv pointer on teardown and probe failure
Quoted 18 lines · Andreas Kemnade
Quoted 15 lines · On Tue, 27 Oct 2026 09:30:42 +0000 Lee Jones <[email protected]> wrote:
twl_probe()allocates the file-scopetwl_privstate withdevm_kzalloc()and checks for a non-NULLtwl_privat entry to reject multiple device instances. However,twl_remove()and the early error paths intwl_probe()never resettwl_privto NULL, a failure when adding MFD child devices bypassestwl_remove()by jumping to free instead of fail, andtwl_remove()tears down the IRQ domain and dummy I2C clients before child devices are unregistered. After devres frees the underlying memory on probe failure or unbind,twl_privremains a dangling pointer, causing subsequent exported API calls to dereference freed memory and blocking future probe attempts with-EBUSY.Unregister DT-populated and MFD child devices at the start of
twl_remove(), ensuremfd_add_devices()failures unwind via fail, and resettwl_privto NULL intwl_remove()and on probe error exit.
Quoted 7 lines
Quoted 4 lines
Fixes 80a97ccd33be ("mfd: twl-core: Collect global variables behind one private structure (global)")Signed-off-by Lee Jones <[email protected]>Hmm, couldn't we use
devm_add_action_or_reset()for resetting thetwl_privpointer? Feels more clean for me.
Nice! Thanks for the suggestion.
To keep this fix minimal for Stable backporting (and avoid resending a 27-patch series), I'll take this one as-is and send a follow-up patch converting twl_priv to use devm_add_action_or_reset().
Signature · 1 line
Lee Jones
Andreas Kemnade
maintainerRe: [PATCH v2 13/27] mfd: twl-core: Clear twl_priv pointer on teardown and probe failure
Quoted 25 lines · On Fri, 30 Oct 2026 10:48:06 +0000 Lee Jones <[email protected]> wrote:
Quoted 23 lines · Andreas Kemnade
Quoted 19 lines · On Tue, 27 Oct 2026 09:30:42 +0000 Lee Jones <[email protected]> wrote:
twl_probe()allocates the file-scopetwl_privstate withdevm_kzalloc()and checks for a non-NULLtwl_privat entry to reject multiple device instances. However,twl_remove()and the early error paths intwl_probe()never resettwl_privto NULL, a failure when adding MFD child devices bypassestwl_remove()by jumping to free instead of fail, andtwl_remove()tears down the IRQ domain and dummy I2C clients before child devices are unregistered. After devres frees the underlying memory on probe failure or unbind,twl_privremains a dangling pointer, causing subsequent exported API calls to dereference freed memory and blocking future probe attempts with-EBUSY.Unregister DT-populated and MFD child devices at the start of
twl_remove(), ensuremfd_add_devices()failures unwind via fail, and resettwl_privto NULL intwl_remove()and on probe error exit.Fixes 80a97ccd33be ("mfd: twl-core: Collect global variables behind one private structure (global)")Signed-off-by Lee Jones <[email protected]>
Quoted 9 lines
Quoted 2 lines
Hmm, couldn't we use
devm_add_action_or_reset()for resetting thetwl_privpointer? Feels more clean for me.Nice! Thanks for the suggestion.
To keep this fix minimal for Stable backporting (and avoid resending a 27-patch series), I'll take this one as-is and send a follow-up patch converting
twl_privto usedevm_add_action_or_reset().
I think the whole patch can be simplified a lot then,
devm_mfd_add_devices can stay.
Regards,
Andreas
Lee Jones
maintainerRe: [PATCH v2 13/27] mfd: twl-core: Clear twl_priv pointer on teardown and probe failure
Quoted 33 lines · Andreas Kemnade
Quoted 30 lines · On Fri, 30 Oct 2026 10:48:06 +0000 Lee Jones <[email protected]> wrote:
Quoted 18 lines · Andreas Kemnade
Quoted 15 lines · On Tue, 27 Oct 2026 09:30:42 +0000 Lee Jones <[email protected]> wrote:
twl_probe()allocates the file-scopetwl_privstate withdevm_kzalloc()and checks for a non-NULLtwl_privat entry to reject multiple device instances. However,twl_remove()and the early error paths intwl_probe()never resettwl_privto NULL, a failure when adding MFD child devices bypassestwl_remove()by jumping to free instead of fail, andtwl_remove()tears down the IRQ domain and dummy I2C clients before child devices are unregistered. After devres frees the underlying memory on probe failure or unbind,twl_privremains a dangling pointer, causing subsequent exported API calls to dereference freed memory and blocking future probe attempts with-EBUSY.Unregister DT-populated and MFD child devices at the start of
twl_remove(), ensuremfd_add_devices()failures unwind via fail, and resettwl_privto NULL intwl_remove()and on probe error exit.Quoted 7 lines
Quoted 4 lines
Fixes 80a97ccd33be ("mfd: twl-core: Collect global variables behind one private structure (global)")Signed-off-by Lee Jones <[email protected]>Hmm, couldn't we use
devm_add_action_or_reset()for resetting thetwl_privpointer? Feels more clean for me.Nice! Thanks for the suggestion.
Quoted 8 lines
Quoted 4 lines
To keep this fix minimal for Stable backporting (and avoid resending a 27-patch series), I'll take this one as-is and send a follow-up patch converting
twl_privto usedevm_add_action_or_reset().I think the whole patch can be simplified a lot then,
devm_mfd_add_devicescan stay.
twl_remove() runs before devres teardown and destroys the IRQ domain, unregisters the dummy I2C clients and clears twl_priv->ready.
So the MFD child devices still need to be unregistered at the start of twl_remove() (alongside of_platform_depopulate()) before the parent transport is torn down beneath them, at least until the rest of twl_remove() is also converted to devm_*().
Signature · 1 line
Lee Jones
Andreas Kemnade
maintainerRe: [PATCH v2 13/27] mfd: twl-core: Clear twl_priv pointer on teardown and probe failure
Quoted 44 lines · On Fri, 30 Oct 2026 11:28:35 +0000 Lee Jones <[email protected]> wrote:
Quoted 33 lines · Andreas Kemnade
Quoted 30 lines · On Fri, 30 Oct 2026 10:48:06 +0000 Lee Jones <[email protected]> wrote:
Quoted 18 lines · Andreas Kemnade
Quoted 15 lines · On Tue, 27 Oct 2026 09:30:42 +0000 Lee Jones <[email protected]> wrote:
twl_probe()allocates the file-scopetwl_privstate withdevm_kzalloc()and checks for a non-NULLtwl_privat entry to reject multiple device instances. However,twl_remove()and the early error paths intwl_probe()never resettwl_privto NULL, a failure when adding MFD child devices bypassestwl_remove()by jumping to free instead of fail, andtwl_remove()tears down the IRQ domain and dummy I2C clients before child devices are unregistered. After devres frees the underlying memory on probe failure or unbind,twl_privremains a dangling pointer, causing subsequent exported API calls to dereference freed memory and blocking future probe attempts with-EBUSY.Unregister DT-populated and MFD child devices at the start of
twl_remove(), ensuremfd_add_devices()failures unwind via fail, and resettwl_privto NULL intwl_remove()and on probe error exit.Quoted 7 lines
Quoted 4 lines
Fixes 80a97ccd33be ("mfd: twl-core: Collect global variables behind one private structure (global)")Signed-off-by Lee Jones <[email protected]>Hmm, couldn't we use
devm_add_action_or_reset()for resetting thetwl_privpointer? Feels more clean for me.Nice! Thanks for the suggestion.
Quoted 8 lines
Quoted 4 lines
To keep this fix minimal for Stable backporting (and avoid resending a 27-patch series), I'll take this one as-is and send a follow-up patch converting
twl_privto usedevm_add_action_or_reset().I think the whole patch can be simplified a lot then,
devm_mfd_add_devicescan stay.
Quoted 8 lines
twl_remove()runs before devres teardown and destroys the IRQ domain, unregisters the dummy I2C clients and clearstwl_priv->ready.So the MFD child devices still need to be unregistered at the start of
twl_remove()(alongsideof_platform_depopulate()) before the parent transport is torn down beneath them, at least until the rest oftwl_remove()is also converted to devm_*().
ok, I merged the two chunks of twl_remove() into one in my head without the lines in between... So you are right.
Regards,
Andreas
Lee Jones
maintainerRe: [PATCH v2 13/27] mfd: twl-core: Clear twl_priv pointer on teardown and probe failure
Quoted 50 lines · Andreas Kemnade
Quoted 47 lines · On Fri, 30 Oct 2026 11:28:35 +0000 Lee Jones <[email protected]> wrote:
Quoted 33 lines · Andreas Kemnade
Quoted 30 lines · On Fri, 30 Oct 2026 10:48:06 +0000 Lee Jones <[email protected]> wrote:
Quoted 18 lines · Andreas Kemnade
Quoted 15 lines · On Tue, 27 Oct 2026 09:30:42 +0000 Lee Jones <[email protected]> wrote:
twl_probe()allocates the file-scopetwl_privstate withdevm_kzalloc()and checks for a non-NULLtwl_privat entry to reject multiple device instances. However,twl_remove()and the early error paths intwl_probe()never resettwl_privto NULL, a failure when adding MFD child devices bypassestwl_remove()by jumping to free instead of fail, andtwl_remove()tears down the IRQ domain and dummy I2C clients before child devices are unregistered. After devres frees the underlying memory on probe failure or unbind,twl_privremains a dangling pointer, causing subsequent exported API calls to dereference freed memory and blocking future probe attempts with-EBUSY.Unregister DT-populated and MFD child devices at the start of
twl_remove(), ensuremfd_add_devices()failures unwind via fail, and resettwl_privto NULL intwl_remove()and on probe error exit.Quoted 7 lines
Quoted 4 lines
Fixes 80a97ccd33be ("mfd: twl-core: Collect global variables behind one private structure (global)")Signed-off-by Lee Jones <[email protected]>Hmm, couldn't we use
devm_add_action_or_reset()for resetting thetwl_privpointer? Feels more clean for me.Nice! Thanks for the suggestion.
Quoted 8 lines
Quoted 4 lines
To keep this fix minimal for Stable backporting (and avoid resending a 27-patch series), I'll take this one as-is and send a follow-up patch converting
twl_privto usedevm_add_action_or_reset().I think the whole patch can be simplified a lot then,
devm_mfd_add_devicescan stay.
twl_remove()runs before devres teardown and destroys the IRQ domain, unregisters the dummy I2C clients and clearstwl_priv->ready.
Quoted 8 lines
Quoted 5 lines
So the MFD child devices still need to be unregistered at the start of
twl_remove()(alongsideof_platform_depopulate()) before the parent transport is torn down beneath them, at least until the rest oftwl_remove()is also converted to devm_*().ok, I merged the two chunks of
twl_remove()into one in my head without the lines in between... So you are right.
No problem. Thanks for your review.
Signature · 1 line
Lee Jones
Lee Jones
maintainerPATCH v2 14/27 mfd: loongson-se: Check return value of dma_set_mask_and_coherent()
In reply to Lee Jonesloongson_se_probe() ignores the return value of dma_set_mask_and_coherent() when requesting a 64-bit DMA mask before allocating coherent DMA memory. Check the return value and abort probe if the platform cannot satisfy the 64-bit DMA mask requirement.
--- drivers/mfd/loongson-se.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/mfd/loongson-se.c +4 -1
@@ -199,7 +199,10 @@ static int loongson_se_probe(struct platform_device *pdev)spin_lock_init (&se->dev_lock);mutex_init (&se->engine_init_lock); -dma_set_mask_and_coherent (dev,DMA_BIT_MASK (64 )); + err =dma_set_mask_and_coherent (dev,DMA_BIT_MASK (64 )); +if (err) +return err; +if (device_property_read_u32 (dev,"dmam_size" , &se->dmam_size))return -ENODEV ;
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 15/27 mfd: loongson-se: Zero-initialise controller command structures
In reply to Lee Jonesloongson_se_init() and loongson_se_init_engine() declare struct loongson_se_controller_cmd on the stack without zero-initialising it and only populate a subset of its fields before loongson_se_send_controller_cmd() writes all eight 32-bit words to the controller MMIO registers. Zero-initialise the stack command structures so that unused words do not leak residual kernel stack contents to the hardware.
--- drivers/mfd/loongson-se.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/mfd/loongson-se.c +2 -2
@@ -93,7 +93,7 @@ struct loongson_se_engine *loongson_se_init_engine(struct device *dev, int id) {struct loongson_se *se =dev_get_drvdata (dev);struct loongson_se_engine *engine = &se->engines[id]; -struct loongson_se_controller_cmd cmd; +struct loongson_se_controller_cmd cmd = {}; engine->se = se; engine->id = id;
@@ -162,7 +162,7 @@ static irqreturn_t se_irq_handler(int irq, void *dev_id)static int loongson_se_init (struct loongson_se *se,dma_addr_t addr,int size) { -struct loongson_se_controller_cmd cmd; +struct loongson_se_controller_cmd cmd = {};int err; cmd.command_id =SE_CMD_START ;
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 16/27 mfd: loongson-se: Reinitialise completions and wait uninterruptibly for commands
In reply to Lee Jonesloongson_se_send_controller_cmd() and loongson_se_send_engine_cmd() wait on se->cmd_completion and engine->completion using wait_for_completion_interruptible() without reinitialising the completion before issuing a new command. Because the hardware has no mechanism to abort an in-flight command once accepted by loongson_se_poll(), returning early on a signal leaves the controller or engine actively processing the shared command registers and DMA buffers in the background, and its subsequent completion interrupt leaves a stale completion token that causes the next command to return prematurely.
Clear any stale pending interrupt status and reinitialise the target completion inside loongson_se_poll() while holding se->dev_lock immediately before notifying the controller, and wait uninterruptibly via wait_for_completion() once loongson_se_poll() confirms the command has been accepted by the hardware.
--- drivers/mfd/loongson-se.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
drivers/mfd/loongson-se.c +14 -5
@@ -37,13 +37,17 @@ struct loongson_se_controller_cmd {u32 info[7 ]; }; -static int loongson_se_poll (struct loongson_se *se,u32 int_bit) +static int loongson_se_poll (struct loongson_se *se,u32 int_bit, +struct completion *completion) {u32 status;int err;spin_lock_irq (&se->dev_lock); +writel (int_bit, se->base +SE_S2LINT_CL ); +reinit_completion (completion); +/* Notify the controller that the engine needs to be started */ writel (int_bit, se->base +SE_L2SINT_SET );
@@ -66,11 +70,13 @@ static int loongson_se_send_controller_cmd(struct loongson_se *se,for (i =0 ; i <SE_SEND_CMD_REG_LEN ; i++)writel (send_cmd[i], se->base +SE_SEND_CMD_REG + i *4 ); - err =loongson_se_poll (se,SE_INT_CONTROLLER ); + err =loongson_se_poll (se,SE_INT_CONTROLLER , &se->cmd_completion);if (err)return err; -return wait_for_completion_interruptible (&se->cmd_completion); +wait_for_completion (&se->cmd_completion); + +return 0 ; }int loongson_se_send_engine_cmd (struct loongson_se_engine *engine)
@@ -80,12 +86,15 @@ int loongson_se_send_engine_cmd(struct loongson_se_engine *engine)* where to obtain engine commands from. Now all we need to * do is notify the controller that the engine needs to be started. */ -int err =loongson_se_poll (engine->se,BIT (engine->id)); +int err =loongson_se_poll (engine->se,BIT (engine->id), + &engine->completion);if (err)return err; -return wait_for_completion_interruptible (&engine->completion); +wait_for_completion (&engine->completion); + +return 0 ; }EXPORT_SYMBOL_GPL (loongson_se_send_engine_cmd);
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 17/27 mfd: loongson-se: Zero-allocate driver data and initialise engine completions in probe
In reply to Lee Jonesloongson_se_probe() allocates struct loongson_se using devm_kmalloc(), leaving the se->engines array uninitialised until child MFD drivers later invoke loongson_se_init_engine(). Meanwhile, hardware interrupts are unmasked and se_irq_handler() is registered during probe. Any engine interrupt arriving before loongson_se_init_engine() runs causes se_irq_handler() to call complete() on an uninitialised completion structure.
Allocate struct loongson_se with devm_kzalloc() and initialise all engine completion structures in loongson_se_probe() before registering interrupt handlers.
--- drivers/mfd/loongson-se.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/mfd/loongson-se.c +3 -2
@@ -106,7 +106,6 @@ struct loongson_se_engine *loongson_se_init_engine(struct device *dev, int id) engine->se = se; engine->id = id; -init_completion (&engine->completion);/* Divide DMA memory equally among all engines */ engine->buffer_size = se->dmam_size /SE_ENGINE_MAX ;
@@ -199,12 +198,14 @@ static int loongson_se_probe(struct platform_device *pdev)int nr_irq, irq, err, i;dma_addr_t paddr; - se =devm_kmalloc (dev,sizeof (*se),GFP_KERNEL ); + se =devm_kzalloc (dev,sizeof (*se),GFP_KERNEL );if (!se)return -ENOMEM ;dev_set_drvdata (dev, se);init_completion (&se->cmd_completion); +for (i =0 ; i <SE_ENGINE_MAX ; i++) +init_completion (&se->engines[i].completion);spin_lock_init (&se->dev_lock);mutex_init (&se->engine_init_lock);
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 18/27 mfd: loongson-se: Prevent out-of-bounds engine access in se_irq_handler()
In reply to Lee Jonesloongson_se_probe() enables all 32 interrupt bits via SE_INT_ALL (0xffffffff), and se_irq_handler() uses the set bit index from the 32-bit status register directly to index se->engines, which only holds SE_ENGINE_MAX (16) entries. Any interrupt bit asserted at index 16 or above triggers an out-of-bounds access on se->engines.
Restrict SE_INT_ALL to GENMASK(SE_ENGINE_MAX - 1, 0) and bounds-check the bit index against SE_ENGINE_MAX before completing the engine in se_irq_handler().
--- drivers/mfd/loongson-se.c | 3 ++- include/linux/mfd/loongson-se.h | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-)
drivers/mfd/loongson-se.c +2 -1
@@ -158,7 +158,8 @@ static irqreturn_t se_irq_handler(int irq, void *dev_id)/* For engines */ while (int_status) { id =__ffs (int_status); -complete (&se->engines[id].completion); +if (id <SE_ENGINE_MAX ) +complete (&se->engines[id].completion); int_status &= ~BIT (id);writel (BIT (id), se->base +SE_S2LINT_CL ); }
include/linux/mfd/loongson-se.h +4 -3
@@ -4,6 +4,8 @@#ifndef __MFD_LOONGSON_SE_H__#define __MFD_LOONGSON_SE_H__ +#include <linux/bits.h> +#define LOONGSON_ENGINE_CMD_TIMEOUT_US 10000#define SE_SEND_CMD_REG 0x0#define SE_SEND_CMD_REG_LEN 0x8
@@ -18,10 +20,9 @@#define SE_L2SINT_STAT 0x98#define SE_L2SINT_SET 0xa0 -#define SE_INT_ALL 0xffffffff -#define SE_INT_CONTROLLER BIT(0) -#define SE_ENGINE_MAX 16 +#define SE_INT_ALL GENMASK(SE_ENGINE_MAX - 1, 0) +#define SE_INT_CONTROLLER BIT(0)#define SE_ENGINE_RNG 1#define SE_CMD_RNG 0x100
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 19/27 mfd: loongson-se: Stop controller and mask interrupts on teardown and shutdown
In reply to Lee Jonesloongson_se_probe() enables hardware interrupts before registering IRQ handlers and starts the controller via loongson_se_init(), but provides no teardown or shutdown handler to stop the controller and mask interrupts. If probe fails after loongson_se_init(), if the driver is unbound, or during a system reboot or kexec transition, the controller remains active with DMA configured to memory that is reclaimed.
Clear pending interrupts before requesting IRQs, unmask interrupts only after IRQ handlers are registered, clean up on failure inside loongson_se_init(), register a devres action after loongson_se_init() succeeds to issue SE_CMD_STOP and mask interrupts on teardown, and wire up the platform driver .shutdown callback.
--- drivers/mfd/loongson-se.c | 51 +++++++++++++++++++++++++++++---- include/linux/mfd/loongson-se.h | 1 + 2 files changed, 46 insertions(+), 6 deletions(-)
drivers/mfd/loongson-se.c +45 -6
@@ -169,6 +169,23 @@ static irqreturn_t se_irq_handler(int irq, void *dev_id)return IRQ_HANDLED ; } +static void loongson_se_stop (void *data) +{ +struct loongson_se *se = data; +struct loongson_se_controller_cmd cmd = {}; + +mutex_lock (&se->engine_init_lock); + + cmd.command_id =SE_CMD_STOP ; +loongson_se_send_controller_cmd (se, &cmd); + +spin_lock_irq (&se->dev_lock); +writel (0 , se->base +SE_S2LINT_EN ); +spin_unlock_irq (&se->dev_lock); + +mutex_unlock (&se->engine_init_lock); +} +static int loongson_se_init (struct loongson_se *se,dma_addr_t addr,int size) {struct loongson_se_controller_cmd cmd = {};
@@ -176,15 +193,23 @@ static int loongson_se_init(struct loongson_se *se, dma_addr_t addr, int size) cmd.command_id =SE_CMD_START ; err =loongson_se_send_controller_cmd (se, &cmd); -if (err) +if (err) { +writel (0 , se->base +SE_S2LINT_EN );return err; + } cmd.command_id =SE_CMD_SET_DMA ; cmd.info[0 ] =lower_32_bits (addr); cmd.info[1 ] =upper_32_bits (addr); cmd.info[2 ] = size; -return loongson_se_send_controller_cmd (se, &cmd); + err =loongson_se_send_controller_cmd (se, &cmd); +if (err) { +loongson_se_stop (se); +return err; + } + +return 0 ; }static const struct mfd_cell engines[] = {
@@ -192,6 +217,13 @@ static const struct mfd_cell engines[] = { { .name = "tpm_loongson" }, }; +static void loongson_se_shutdown (struct platform_device *pdev) +{ +struct loongson_se *se =platform_get_drvdata (pdev); + +loongson_se_stop (se); +} +static int loongson_se_probe (struct platform_device *pdev) {struct device *dev = &pdev->dev;
@@ -225,12 +257,12 @@ static int loongson_se_probe(struct platform_device *pdev)if (IS_ERR (se->base))return PTR_ERR (se->base); -writel (SE_INT_ALL , se->base +SE_S2LINT_EN ); - nr_irq =platform_irq_count (pdev);if (nr_irq <=0 )return -ENODEV ; +writel (SE_INT_ALL , se->base +SE_S2LINT_CL ); +for (i =0 ; i < nr_irq; i++) { irq =platform_get_irq (pdev, i); err =devm_request_irq (dev, irq, se_irq_handler,0 ,"loongson-se" , se);
@@ -238,10 +270,16 @@ static int loongson_se_probe(struct platform_device *pdev)dev_err (dev,"failed to request IRQ: %d\n" , irq); } +writel (SE_INT_ALL , se->base +SE_S2LINT_EN ); + err =loongson_se_init (se, paddr, se->dmam_size);if (err)return err; + err =devm_add_action_or_reset (dev, loongson_se_stop, se); +if (err) +return err; +return devm_mfd_add_devices (dev,PLATFORM_DEVID_NONE , engines,ARRAY_SIZE (engines),NULL ,0 ,NULL ); }
@@ -253,8 +291,9 @@ static const struct acpi_device_id loongson_se_acpi_match[] = {MODULE_DEVICE_TABLE (acpi, loongson_se_acpi_match);static struct platform_driver loongson_se_driver = { - .probe = loongson_se_probe, - .driver = { + .probe = loongson_se_probe, + .shutdown = loongson_se_shutdown, + .driver = { .name ="loongson-se" , .acpi_match_table = loongson_se_acpi_match, },
include/linux/mfd/loongson-se.h +1 -0
@@ -11,6 +11,7 @@#define SE_SEND_CMD_REG_LEN 0x8/* Controller command ID */ #define SE_CMD_START 0x0 +#define SE_CMD_STOP 0x1#define SE_CMD_SET_DMA 0x3#define SE_CMD_SET_ENGINE_CMDBUF 0x4
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 20/27 mfd: loongson-se: Validate engine ID, DMA buffer size, and IRQ registration
In reply to Lee JonesHarden input and error checking in loongson_se_init_engine() and loongson_se_probe():
- Reject out-of-bounds engine IDs and engine 0 in loongson_se_init_engine() before indexing se->engines.
- Store dmam_size as u32 to match device_property_read_u32() and verify that it is large enough to hold the command buffers of all engines within engine 0's data region.
- Propagate negative error codes from platform_irq_count(), platform_get_irq(), and devm_request_irq() instead of ignoring IRQ registration failures or masking -EPROBE_DEFER.
--- drivers/mfd/loongson-se.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)
drivers/mfd/loongson-se.c +18 -5
@@ -26,7 +26,7 @@ struct loongson_se {struct completion cmd_completion;void *dmam_base; -int dmam_size; +u32 dmam_size;struct mutex engine_init_lock;struct loongson_se_engine engines[SE_ENGINE_MAX ];
@@ -101,8 +101,13 @@ EXPORT_SYMBOL_GPL(loongson_se_send_engine_cmd);struct loongson_se_engine *loongson_se_init_engine (struct device *dev,int id) {struct loongson_se *se =dev_get_drvdata (dev); -struct loongson_se_engine *engine = &se->engines[id];struct loongson_se_controller_cmd cmd = {}; +struct loongson_se_engine *engine; + +if (id <=0 || id >=SE_ENGINE_MAX ) +return NULL ; + + engine = &se->engines[id]; engine->se = se; engine->id = id;
@@ -186,7 +191,7 @@ static void loongson_se_stop(void *data)mutex_unlock (&se->engine_init_lock); } -static int loongson_se_init (struct loongson_se *se,dma_addr_t addr,int size) +static int loongson_se_init (struct loongson_se *se,dma_addr_t addr,u32 size) {struct loongson_se_controller_cmd cmd = {};int err;
@@ -249,6 +254,9 @@ static int loongson_se_probe(struct platform_device *pdev)if (device_property_read_u32 (dev,"dmam_size" , &se->dmam_size))return -ENODEV ; +if ((se->dmam_size /SE_ENGINE_MAX ) < (SE_ENGINE_MAX *2 *SE_ENGINE_CMD_SIZE )) +return -EINVAL ; + se->dmam_base =dmam_alloc_coherent (dev, se->dmam_size, &paddr,GFP_KERNEL );if (!se->dmam_base)return -ENOMEM ;
@@ -258,16 +266,21 @@ static int loongson_se_probe(struct platform_device *pdev)return PTR_ERR (se->base); nr_irq =platform_irq_count (pdev); -if (nr_irq <=0 ) +if (!nr_irq)return -ENODEV ; +if (nr_irq <0 ) +return nr_irq;writel (SE_INT_ALL , se->base +SE_S2LINT_CL );for (i =0 ; i < nr_irq; i++) { irq =platform_get_irq (pdev, i); +if (irq <0 ) +return irq; + err =devm_request_irq (dev, irq, se_irq_handler,0 ,"loongson-se" , se);if (err) -dev_err (dev,"failed to request IRQ: %d\n" , irq); +return dev_err_probe (dev, err,"Failed to request IRQ: %d\n" , irq); }writel (SE_INT_ALL , se->base +SE_S2LINT_EN );
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 21/27 mfd: abx500-core: Unregister device ops on devm release
In reply to Lee Jonesabx500_register_ops() allocates struct abx500_device_entry via devm_kzalloc() and links it into the global abx500_list without registering a devres cleanup action. If a caller's probe function subsequently fails, devres frees the entry while it remains linked in abx500_list, causing use-after-free and list corruption on later traversals.
Register a devm_add_action_or_reset() callback in abx500_register_ops() to remove the entry from abx500_list before devres frees it.
--- drivers/mfd/abx500-core.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-)
drivers/mfd/abx500-core.c +18 -11
@@ -5,6 +5,7 @@* Author: Mattias Wallin <[email protected]> */ +#include <linux/device.h> #include <linux/list.h> #include <linux/slab.h> #include <linux/err.h>
@@ -33,6 +34,21 @@ static void lookup_ops(struct device *dev, struct abx500_ops **ops) } } +void abx500_remove_ops (struct device *dev) +{ +struct abx500_device_entry *dev_entry, *tmp; + +list_for_each_entry_safe (dev_entry, tmp, &abx500_list, list) +if (dev_entry->dev == dev) +list_del (&dev_entry->list); +} +EXPORT_SYMBOL (abx500_remove_ops); + +static void abx500_unregister_ops (void *data) +{ +abx500_remove_ops (data); +} +int abx500_register_ops (struct device *dev,struct abx500_ops *ops) {struct abx500_device_entry *dev_entry;
@@ -45,20 +61,11 @@ int abx500_register_ops(struct device *dev, struct abx500_ops *ops)memcpy (&dev_entry->ops, ops,sizeof (*ops));list_add_tail (&dev_entry->list, &abx500_list); -return 0 ; + +return devm_add_action_or_reset (dev, abx500_unregister_ops, dev); }EXPORT_SYMBOL (abx500_register_ops); -void abx500_remove_ops (struct device *dev) -{ -struct abx500_device_entry *dev_entry, *tmp; - -list_for_each_entry_safe (dev_entry, tmp, &abx500_list, list) -if (dev_entry->dev == dev) -list_del (&dev_entry->list); -} -EXPORT_SYMBOL (abx500_remove_ops); -int abx500_set_register_interruptible (struct device *dev,u8 bank,u8 reg,u8 value) {
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 22/27 mfd: ab8500-core: Register sysfs attribute groups via devm_device_add_group()
In reply to Lee Jonesab8500_probe() registers up to two sysfs attribute groups with sysfs_create_group(). On AB8505 and AB9540 (cut 2.0 and later), the second sysfs_create_group() call unconditionally overwrites the return value of the first, and if the second call fails, ab8500_probe() returns an error without removing the first group. Because the driver context is freed and driver data is cleared on probe failure, reading any leaked sysfs attribute dereferences a NULL ab8500 pointer in get_register_interruptible().
Register the attribute groups via devm_device_add_group() and only register the second group when the first succeeds so that devres automatically cleans up registered sysfs attributes on probe failure.
--- drivers/mfd/ab8500-core.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
drivers/mfd/ab8500-core.c +4 -7
@@ -1232,16 +1232,13 @@ static int ab8500_probe(struct platform_device *pdev)if (((is_ab8505 (ab8500) ||is_ab9540 (ab8500)) && ab8500->chip_id >=AB8500_CUT2P0 ) ||is_ab8540 (ab8500)) - ret =sysfs_create_group (&ab8500->dev->kobj, - &ab9540_attr_group); + ret =devm_device_add_group (ab8500->dev, &ab9540_attr_group);else - ret =sysfs_create_group (&ab8500->dev->kobj, - &ab8500_attr_group); + ret =devm_device_add_group (ab8500->dev, &ab8500_attr_group);if ((is_ab8505 (ab8500) ||is_ab9540 (ab8500)) && - ab8500->chip_id >=AB8500_CUT2P0 ) - ret =sysfs_create_group (&ab8500->dev->kobj, - &ab8505_attr_group); + ab8500->chip_id >=AB8500_CUT2P0 && !ret) + ret =devm_device_add_group (ab8500->dev, &ab8505_attr_group);if (ret)dev_err (ab8500->dev,"error creating sysfs entries\n" );
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 23/27 mfd: tps65217: Manage IRQ domain teardown via devres
In reply to Lee Jonestps65217_irq_init() creates tps->irq_domain with an unmanaged call and requests the parent interrupt via devm_request_threaded_irq(), while tps65217_remove() manually disposes mappings and removes tps->irq_domain. Because tps65217_remove() executes before devres teardown, the IRQ domain is destroyed while tps65217_irq_thread() and child MFD devices are still active, allowing a concurrent interrupt to dereference a freed or NULL domain. Furthermore, if probe fails after irq_domain_create_linear() or if tps65217_irq_init() fails, the IRQ domain is leaked, and if the device is probed without an interrupt (client->irq == 0), tps65217_remove() unconditionally passes a NULL tps->irq_domain to irq_domain_remove().
Register a devres action to tear down the IRQ domain in tps65217_irq_init() prior to devm_request_threaded_irq(), propagate errors from tps65217_irq_init() in tps65217_probe(), and drop tps65217_remove().
--- drivers/mfd/tps65217.c | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-)
drivers/mfd/tps65217.c +22 -18
@@ -146,6 +146,21 @@ static const struct irq_domain_ops tps65217_irq_domain_ops = { .map = tps65217_irq_map, }; +static void tps65217_irq_exit (void *data) +{ +struct tps65217 *tps = data; +unsigned int virq; + +for (int i =0 ; i <TPS65217_NUM_IRQ ; i++) { + virq =irq_find_mapping (tps->irq_domain, i); +if (virq) +irq_dispose_mapping (virq); + } + +irq_domain_remove (tps->irq_domain); + tps->irq_domain =NULL ; +} +static int tps65217_irq_init (struct tps65217 *tps,int irq) {int ret;
@@ -165,6 +180,10 @@ static int tps65217_irq_init(struct tps65217 *tps, int irq)return -ENOMEM ; } + ret =devm_add_action_or_reset (tps->dev, tps65217_irq_exit, tps); +if (ret) +return ret; + ret =devm_request_threaded_irq (tps->dev, irq,NULL , tps65217_irq_thread,IRQF_ONESHOT ,"tps65217-irq" , tps);
@@ -333,7 +352,9 @@ static int tps65217_probe(struct i2c_client *client) }if (client->irq) { -tps65217_irq_init (tps, client->irq); + ret =tps65217_irq_init (tps, client->irq); +if (ret) +return ret; }else {int i;
@@ -373,22 +394,6 @@ static int tps65217_probe(struct i2c_client *client)return 0 ; } -static void tps65217_remove (struct i2c_client *client) -{ -struct tps65217 *tps =i2c_get_clientdata (client); -unsigned int virq; -int i; - -for (i =0 ; i <TPS65217_NUM_IRQ ; i++) { - virq =irq_find_mapping (tps->irq_domain, i); -if (virq) -irq_dispose_mapping (virq); - } - -irq_domain_remove (tps->irq_domain); - tps->irq_domain =NULL ; -} -static const struct i2c_device_id tps65217_id_table[] = { {"tps65217" ,TPS65217 }, {/* sentinel */ }
@@ -402,7 +407,6 @@ static struct i2c_driver tps65217_driver = { }, .id_table = tps65217_id_table, .probe = tps65217_probe, - .remove = tps65217_remove, };static int __init tps65217_init (void )
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 24/27 mfd: mt6397: Manage IRQ domain lifecycle with devres
In reply to Lee Jonesmt6397_irq_init() and mt6358_irq_init() create chip->irq_domain with irq_domain_create_linear() before registering the parent interrupt via devm_request_threaded_irq(). If devm_mfd_add_devices() fails in mt6397_probe(), irq_domain_remove() is called before devres frees the threaded IRQ and without disposing child IRQ mappings, allowing an in-flight interrupt handler or late IRQ lookup to dereference the removed domain. Conversely, when the driver is unbound, the IRQ mappings and domain are never removed at all.
Register a devres action immediately after irq_domain_create_linear() to dispose all mapped hardware interrupts and remove chip->irq_domain so that the domain is torn down after child MFD devices and the managed IRQ handler have been released on both probe failure and unbind.
--- drivers/mfd/mt6358-irq.c | 17 ++++++++++++++++- drivers/mfd/mt6397-core.c | 4 +--- drivers/mfd/mt6397-irq.c | 16 +++++++++++++++- 3 files changed, 32 insertions(+), 5 deletions(-)
drivers/mfd/mt6358-irq.c +16 -1
@@ -224,6 +224,17 @@ static const struct irq_domain_ops mt6358_irq_domain_ops = { .xlate = irq_domain_xlate_twocell, }; +static void mt6358_irq_domain_remove (void *data) +{ +struct mt6397_chip *chip = data; +struct pmic_irq_data *irqd = chip->irq_data; + +for (unsigned int hwirq =0 ; hwirq < irqd->num_pmic_irqs; hwirq++) +irq_dispose_mapping (irq_find_mapping (chip->irq_domain, hwirq)); + +irq_domain_remove (chip->irq_domain); +} +int mt6358_irq_init (struct mt6397_chip *chip) {int i, j, ret;
@@ -279,13 +290,17 @@ int mt6358_irq_init(struct mt6397_chip *chip)return -ENODEV ; } + ret =devm_add_action_or_reset (chip->dev, mt6358_irq_domain_remove, + chip); +if (ret) +return ret; + ret =devm_request_threaded_irq (chip->dev, chip->irq,NULL , mt6358_irq_handler,IRQF_ONESHOT , mt6358_irq_chip.name, chip);if (ret) {dev_err (chip->dev,"Failed to register IRQ=%d, ret=%d\n" , chip->irq, ret); -irq_domain_remove (chip->irq_domain);return ret; }
drivers/mfd/mt6397-core.c +1 -3
@@ -395,10 +395,8 @@ static int mt6397_probe(struct platform_device *pdev) ret =devm_mfd_add_devices (&pdev->dev,PLATFORM_DEVID_NONE , pmic_core->cells, pmic_core->cell_size,NULL ,0 , pmic->irq_domain); -if (ret) { -irq_domain_remove (pmic->irq_domain); +if (ret)dev_err (&pdev->dev,"failed to add child devices: %d\n" , ret); - }return ret; }
drivers/mfd/mt6397-irq.c +15 -1
@@ -169,6 +169,16 @@ static int mt6397_irq_pm_notifier(struct notifier_block *notifier,return NOTIFY_DONE ; } +static void mt6397_irq_domain_remove (void *data) +{ +struct mt6397_chip *chip = data; + +for (int hwirq =0 ; hwirq <MT6397_IRQ_NR ; hwirq++) +irq_dispose_mapping (irq_find_mapping (chip->irq_domain, hwirq)); + +irq_domain_remove (chip->irq_domain); +} +int mt6397_irq_init (struct mt6397_chip *chip) {int ret;
@@ -223,13 +233,17 @@ int mt6397_irq_init(struct mt6397_chip *chip)return -ENOMEM ; } + ret =devm_add_action_or_reset (chip->dev, mt6397_irq_domain_remove, + chip); +if (ret) +return ret; + ret =devm_request_threaded_irq (chip->dev, chip->irq,NULL , mt6397_irq_thread,IRQF_ONESHOT ,"mt6397-pmic" , chip);if (ret) {dev_err (chip->dev,"failed to register irq=%d; err: %d\n" , chip->irq, ret); -irq_domain_remove (chip->irq_domain);return ret; }
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Re: [PATCH v2 24/27] mfd: mt6397: Manage IRQ domain lifecycle with devres
Quoted 15 lines · Lee Jones
mt6397_irq_init()andmt6358_irq_init()createchip->irq_domainwithirq_domain_create_linear()before registering the parent interrupt viadevm_request_threaded_irq(). Ifdevm_mfd_add_devices()fails inmt6397_probe(),irq_domain_remove()is called before devres frees the threaded IRQ and without disposing child IRQ mappings, allowing an in-flight interrupt handler or late IRQ lookup to dereference the removed domain. Conversely, when the driver is unbound, the IRQ mappings and domain are never removed at all.Register a devres action immediately after
irq_domain_create_linear()to dispose all mapped hardware interrupts and removechip->irq_domainso that the domain is torn down after child MFD devices and the managed IRQ handler have been released on both probe failure and unbind.
Quoted 4 lines
Fixes 6df8dd5c185b ("mfd: Add support for the MediaTek MT6397 PMIC")Signed-off-by Lee Jones <[email protected]>
Lee Jones
maintainerPATCH v2 25/27 mfd: mt6397: Unregister PM notifier on probe failure and unbind
In reply to Lee Jonesmt6397_irq_init() registers a power management notifier block embedded in the devres-managed struct mt6397_chip via register_pm_notifier(), but never unregisters it if mt6397_probe() later fails or when the driver is unbound. Once devres frees struct mt6397_chip, the dangling notifier block corrupts the global PM notifier chain and causes a use-after-free on subsequent suspend or resume transitions.
Check the return value of register_pm_notifier() and register a devres action to unregister the PM notifier on probe failure or device unbind.
--- drivers/mfd/mt6397-irq.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
drivers/mfd/mt6397-irq.c +13 -2
@@ -179,6 +179,13 @@ static void mt6397_irq_domain_remove(void *data)irq_domain_remove (chip->irq_domain); } +static void mt6397_unregister_pm_notifier (void *data) +{ +struct mt6397_chip *chip = data; + +unregister_pm_notifier (&chip->pm_nb); +} +int mt6397_irq_init (struct mt6397_chip *chip) {int ret;
@@ -247,6 +254,10 @@ int mt6397_irq_init(struct mt6397_chip *chip)return ret; } -register_pm_notifier (&chip->pm_nb); -return 0 ; + ret =register_pm_notifier (&chip->pm_nb); +if (ret) +return ret; + +return devm_add_action_or_reset (chip->dev, + mt6397_unregister_pm_notifier, chip); }
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Re: [PATCH v2 25/27] mfd: mt6397: Unregister PM notifier on probe failure and unbind
Quoted 8 lines · Lee Jones
mt6397_irq_init()registers a power management notifier block embedded in the devres-managed structmt6397_chipviaregister_pm_notifier(), but never unregisters it ifmt6397_probe()later fails or when the driver is unbound. Once devres frees structmt6397_chip, the dangling notifier block corrupts the global PM notifier chain and causes a use-after-free on subsequent suspend or resume transitions.
Quoted 7 lines
Check the return value of
register_pm_notifier()and register a devres action to unregister the PM notifier on probe failure or device unbind.Fixes 4e2e7cfec13a ("mfd: mt6397: Modify suspend/resume behavior")Signed-off-by Lee Jones <[email protected]>
Though in practice, if one hits this then they have bigger problems.
Lee Jones
maintainerPATCH v2 26/27 i2c: cgbc: Propagate hardware errors when polling read status
In reply to Lee JonesIn cgbc_i2c_xfer_msg(), read_poll_timeout() uses the same ret variable to store the polled return value of cgbc_i2c_get_status() and the timeout result of the macro itself. When cgbc_i2c_get_status() returns a negative error code (such as -EIO on an I2C NACK), the loop condition ret != CGBC_I2C_STAT_BUSY evaluates to true and read_poll_timeout() overwrites ret with 0. The driver then issues CGBC_I2C_CMD_DATA, which completes with CGBC_STATUS_CMD_READY without populating msg->buf, leaving uninitialised heap memory in the caller's read buffer.
Store the polled status in a separate variable, verify that it equals CGBC_I2C_STAT_DAT, and check that CGBC_I2C_CMD_DATA returns the expected number of bytes.
--- drivers/i2c/busses/i2c-cgbc.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-)
drivers/i2c/busses/i2c-cgbc.c +21 -6
@@ -205,9 +205,9 @@ static int cgbc_i2c_xfer_msg(struct i2c_adapter *adap)struct cgbc_device_data *cgbc = i2c->cgbc;struct i2c_msg *msg = i2c->msg;u8 cmd[CGBC_I2C_CMD_SIZE ]; -int ret, max_len, len, i; +int ret, status, max_len, len, i;unsigned int cmd_len; -u8 cmd_data; +u8 cmd_data, data_len;struct cgbc_i2c_transfer xfer = { .bus_id = algo_data->bus_id,
@@ -266,17 +266,32 @@ static int cgbc_i2c_xfer_msg(struct i2c_adapter *adap)if (ret) goto err; - ret =read_poll_timeout (cgbc_i2c_get_status, ret, - ret !=CGBC_I2C_STAT_BUSY ,0 , + ret =read_poll_timeout (cgbc_i2c_get_status, status, + status !=CGBC_I2C_STAT_BUSY ,0 ,2 * algo_data->read_maxtime_us, false, adap); -if (ret <0 ) +if (ret) goto err; +if (status <0 ) { + ret = status; + goto err; + } + +if (status !=CGBC_I2C_STAT_DAT ) { + ret = -EIO ; + goto err; + } + cmd_data =CGBC_I2C_CMD_DATA | algo_data->bus_id; ret =cgbc_command (cgbc, &cmd_data,sizeof (cmd_data), - msg->buf + i2c->pos, len,NULL ); + msg->buf + i2c->pos, len, &data_len);if (ret) goto err; + +if (data_len != len) { + ret = -EIO ; + goto err; + } }if (len == (msg->len - i2c->pos)) {
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog
Lee Jones
maintainerPATCH v2 27/27 mfd: ti_am335x_tscadc: Remove redundant ti_tscadc_dev_get() helper
In reply to Lee JonesNow that ti_tscadc_dev_get() is a single-line wrapper around dev_get_drvdata(p->dev.parent), remove the bespoke helper and retrieve the parent driver data directly in the child ADC and touchscreen drivers.
--- drivers/iio/adc/ti_am335x_adc.c | 2 +- drivers/input/touchscreen/ti_am335x_tsc.c | 2 +- include/linux/mfd/ti_am335x_tscadc.h | 5 ----- 3 files changed, 2 insertions(+), 7 deletions(-)
drivers/iio/adc/ti_am335x_adc.c +1 -1
@@ -636,7 +636,7 @@ static int tiadc_probe(struct platform_device *pdev) adc_dev =iio_priv (indio_dev); - adc_dev->mfd_tscadc =ti_tscadc_dev_get (pdev); + adc_dev->mfd_tscadc =dev_get_drvdata (pdev->dev.parent);tiadc_parse_dt (pdev, adc_dev); indio_dev->name =dev_name (&pdev->dev);
drivers/input/touchscreen/ti_am335x_tsc.c +1 -1
@@ -418,7 +418,7 @@ static int titsc_probe(struct platform_device *pdev) {struct titsc *ts_dev;struct input_dev *input_dev; -struct ti_tscadc_dev *tscadc_dev =ti_tscadc_dev_get (pdev); +struct ti_tscadc_dev *tscadc_dev =dev_get_drvdata (pdev->dev.parent);int err;/* Allocate memory for device */
include/linux/mfd/ti_am335x_tscadc.h +0 -5
@@ -175,11 +175,6 @@ struct ti_tscadc_dev {struct adc_device *adc; }; -static inline struct ti_tscadc_dev *ti_tscadc_dev_get (struct platform_device *p) -{ -return dev_get_drvdata (p->dev.parent); -} -static inline bool ti_adc_with_touchscreen (struct ti_tscadc_dev *tscadc) { return of_device_is_compatible(tscadc->dev->of_node,
Signature · 1 line
2.56.0.307.g5fe7bd680c-goog