linux-kernel - Re: [PATCH] usb: dwc2: fix a devres leak in hw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <0473d8b1-aa99-2458-7993-4320f1178e5b@foss.st.com>
Date:   Thu, 16 Mar 2023 09:45:35 +0100
From:   Fabrice Gasnier <fabrice.gasnier@...s.st.com>
To:     <hminas@...opsys.com>, <gregkh@...uxfoundation.org>,
        <maz@...nel.org>, <m.szyprowski@...sung.com>
CC:     <linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-stm32@...md-mailman.stormreply.com>,
        <amelie.delaunay@...s.st.com>, <alexandre.torgue@...s.st.com>
Subject: Re: [PATCH] usb: dwc2: fix a devres leak in hw_enable upon suspend
 resume

On 3/15/23 17:32, Fabrice Gasnier wrote:
> Each time the platform goes to low power, PM suspend / resume routines
> call: __dwc2_lowlevel_hw_enable -> devm_add_action_or_reset().
> This adds a new devres each time.
> This may also happen at runtime, as dwc2_lowlevel_hw_enable() can be
> called from udc_start().
> 
> This can be seen with tracing:
> - echo 1 > /sys/kernel/debug/tracing/events/dev/devres_log/enable
> - go to low power
> - cat /sys/kernel/debug/tracing/trace
> 
> A new "ADD" entry is found upon each low power cycle:
> ... devres_log: 49000000.usb-otg ADD 82a13bba devm_action_release (8 bytes)
> ... devres_log: 49000000.usb-otg ADD 49889daf devm_action_release (8 bytes)
> ...
> 
> A second issue is addressed here:
> - regulator_bulk_enable() is called upon each PM cycle (suspend/resume).
> - regulator_bulk_disable() never gets called.
> 
> So the reference count for these regulators constantly increase, by one
> upon each low power cycle, due to missing regulator_bulk_disable() call
> in __dwc2_lowlevel_hw_disable().
> 
> The original fix that introduced the devm_add_action_or_reset() call,
> fixed an issue during probe, that happens due to other errors in
> dwc2_driver_probe() -> dwc2_core_reset(). Then the probe fails without
> disabling regulators, when dr_mode == USB_DR_MODE_PERIPHERAL.
> 
> Rather fix the error path: disable all the low level hardware in the
> error path, by using the "hsotg->ll_hw_enabled" flag. Checking dr_mode
> has been introduced to avoid a dual call to dwc2_lowlevel_hw_disable().
> "ll_hw_enabled" should achieve the same (and is used currently in the
> remove() routine).
> 
> Fixes: 54c196060510 ("usb: dwc2: Always disable regulators on driver teardown")
> Fixes: 33a06f1300a7 ("usb: dwc2: Fix error path in gadget registration")
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@...s.st.com>
---

Hi all,

Please ignore this patch, I just sent a V2 to replace it.

Best Regards,
Fabrice