| Message ID | 2440edae7ca8534628cdbaf559ded288f2998178.1701276806.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | reset: Fix crash when freeing non-existent optional resets | expand |
On Wed, 29 Nov 2023 17:55:33 +0100, Geert Uytterhoeven wrote: > When obtaining one or more optional resets, non-existent resets are > stored as NULL pointers, and all related error and cleanup paths need to > take this into account. > > Currently only reset_control_put() and reset_control_bulk_put() > get this right. All of __reset_control_bulk_get(), > of_reset_control_array_get(), and reset_control_array_put() lack the > proper checking, causing NULL pointer dereferences on failure or > release. > > [...] Applied to reset/fixes, thanks! [1/1] reset: Fix crash when freeing non-existent optional resets https://git.pengutronix.de/cgit/pza/linux/commit/?id=4a6756f56bcf regards Philipp
diff --git a/drivers/reset/core.c b/drivers/reset/core.c index 7ece6a8e9858555d..4d5a78d3c085bc76 100644 --- a/drivers/reset/core.c +++ b/drivers/reset/core.c @@ -807,6 +807,9 @@ static void __reset_control_put_internal(struct reset_control *rstc) { lockdep_assert_held(&reset_list_mutex); + if (IS_ERR_OR_NULL(rstc)) + return; + kref_put(&rstc->refcnt, __reset_control_release); } @@ -1017,11 +1020,8 @@ EXPORT_SYMBOL_GPL(reset_control_put); void reset_control_bulk_put(int num_rstcs, struct reset_control_bulk_data *rstcs) { mutex_lock(&reset_list_mutex); - while (num_rstcs--) { - if (IS_ERR_OR_NULL(rstcs[num_rstcs].rstc)) - continue; + while (num_rstcs--) __reset_control_put_internal(rstcs[num_rstcs].rstc); - } mutex_unlock(&reset_list_mutex); } EXPORT_SYMBOL_GPL(reset_control_bulk_put);
When obtaining one or more optional resets, non-existent resets are stored as NULL pointers, and all related error and cleanup paths need to take this into account. Currently only reset_control_put() and reset_control_bulk_put() get this right. All of __reset_control_bulk_get(), of_reset_control_array_get(), and reset_control_array_put() lack the proper checking, causing NULL pointer dereferences on failure or release. Fix this by moving the existing check from reset_control_bulk_put() to __reset_control_put_internal(), so it applies to all callers. The double check in reset_control_put() doesn't hurt. Fixes: 17c82e206d2a3cd8 ("reset: Add APIs to manage array of resets") Fixes: 48d71395896d54ee ("reset: Add reset_control_bulk API") Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- Crash seen on the Renesas R-Car S4-based Spider development board, using a (buggy) out-of-tree patch[1]. As the (mutually-exclusive) PCIe host and endpoint nodes share the same reset, devm_reset_control_bulk_get_optional_exclusive() fails, and crashes in the error path: /soc/pcie@e65d0000 requests exclusive control over reset pwr shared with /soc/pcie-ep@e65d0000 on /soc/clock-controller@e6150000 Unable to handle kernel NULL pointer dereference at virtual address 000000000000001c Mem abort info: ESR = 0x0000000096000005 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 FSC = 0x05: level 1 translation fault Data abort info: ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000 CM = 0, WnR = 0, TnD = 0, TagAccess = 0 GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 user pgtable: 4k pages, 39-bit VAs, pgdp=00000004810b4000 [000000000000001c] pgd=0000000000000000, p4d=0000000000000000, pud=0000000000000000 Internal error: Oops: 0000000096000005 [#1] SMP pstate: 00400005 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : __reset_control_put_internal+0x90/0xc8 lr : __reset_control_bulk_get+0xd4/0xd8 sp : ffffffc08174ba20 x29: ffffffc08174ba20 x28: ffffff8440010cd0 x27: ffffff8440a6f8c0 x26: 0000000000000001 x25: 0000000000000001 x24: 0000000000000000 x23: ffffff84409d4410 x22: ffffff8440a6fe48 x21: ffffffc080e8fae0 x20: 0000000000000005 x19: 0000000000000000 x18: 0000000000000000 x17: 6168732072777020 x16: 7465736572207265 x15: 766f206c6f72746e x14: 6f63206576697375 x13: 3030303035313665 x12: 4072656c6c6f7274 x11: 6e6f632d6b636f6c x10: 632f636f732f206e x9 : 6c63786520737473 x8 : 6575716572203030 x7 : 205d383432323136 x6 : ffffffc080e8fae0 x5 : ffffffc080e8fae0 x4 : 0000000000000004 x3 : 0000000000000000 x2 : 000000000000001c x1 : 00000000ffffffff x0 : 000000000000001c Call trace: __reset_control_put_internal+0x90/0xc8 __reset_control_bulk_get+0xd4/0xd8 __devm_reset_control_bulk_get+0x78/0xcc dw_pcie_get_resources+0x2e8/0x344 dw_pcie_host_init+0x64/0x538 rcar_gen4_pcie_probe+0x1a8/0x1ec platform_probe+0x68/0xb8 really_probe+0x140/0x278 __driver_probe_device+0xf4/0x10c driver_probe_device+0x50/0x100 __device_attach_driver+0xb0/0xd0 bus_for_each_drv+0xa8/0xd0 __device_attach_async_helper+0x70/0xc4 async_run_entry_fn+0x38/0x108 process_scheduled_works+0x1c4/0x270 worker_thread+0x1fc/0x26c kthread+0xbc/0xcc ret_from_fork+0x10/0x20 Code: 97f88daf 14000009 91007262 12800001 (b8610041) ---[ end trace 0000000000000000 ]--- [1] "[PATCH v3] reset: Exclusive resets must be dedicated to a single hardware block" https://lore.kernel.org/r/20181113133520.20889-1-geert+renesas@glider.be --- drivers/reset/core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)