* Re: [PATCH] ALSA: pcmtest: fix reference leak on failed device registration [not found] ` <87h5padnof.wl-tiwai@suse.de> @ 2026-04-17 10:52 ` Takashi Iwai 2026-04-17 11:30 ` Guangshuo Li 2026-04-23 5:19 ` Jiri Slaby 0 siblings, 2 replies; 4+ messages in thread From: Takashi Iwai @ 2026-04-17 10:52 UTC (permalink / raw) To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich Cc: Guangshuo Li, linux-kernel, driver-core On Fri, 17 Apr 2026 09:57:20 +0200, Takashi Iwai wrote: > > On Wed, 15 Apr 2026 21:31:38 +0200, > Guangshuo Li wrote: > > > > When platform_device_register() fails in mod_init(), the embedded struct > > device in pcmtst_pdev has already been initialized by > > device_initialize(), but the failure path returns the error without > > dropping the device reference for the current platform device: > > > > mod_init() > > -> platform_device_register(&pcmtst_pdev) > > -> device_initialize(&pcmtst_pdev.dev) > > -> setup_pdev_dma_masks(&pcmtst_pdev) > > -> platform_device_add(&pcmtst_pdev) > > > > This leads to a reference leak when platform_device_register() fails. > > Fix this by calling platform_device_put() before returning the error. > > > > The issue was identified by a static analysis tool I developed and > > confirmed by manual review. > > > > Fixes: 315a3d57c64c5 ("ALSA: Implement the new Virtual PCM Test Driver") > > Cc: stable@vger.kernel.org > > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com> > > Thanks, applied now. ... and now I looked through the whole tree, and noticed that the majority of callers of platform_device_register() don't care the error cases without calling platform_device_put(). There are over a hundred callers of platform_device_register() while only 5 or so are doing the proper cleanup at the error. Judging from the numbers above, it might be better to change the behavior of platform_device_register() itself to call *_put() at the error case internally. Or, if we keep the current behavior, at least we should properly document it. Greg, Rafael, Danilo, what do you think? thanks, Takashi ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ALSA: pcmtest: fix reference leak on failed device registration 2026-04-17 10:52 ` [PATCH] ALSA: pcmtest: fix reference leak on failed device registration Takashi Iwai @ 2026-04-17 11:30 ` Guangshuo Li 2026-04-23 5:19 ` Jiri Slaby 1 sibling, 0 replies; 4+ messages in thread From: Guangshuo Li @ 2026-04-17 11:30 UTC (permalink / raw) To: Takashi Iwai Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, linux-kernel, driver-core Hi Takashi, Thanks for reviewing. On Fri, 17 Apr 2026 at 18:52, Takashi Iwai <tiwai@suse.de> wrote: > > > ... and now I looked through the whole tree, and noticed that the > majority of callers of platform_device_register() don't care the error > cases without calling platform_device_put(). There are over a hundred > callers of platform_device_register() while only 5 or so are doing the > proper cleanup at the error. > > Judging from the numbers above, it might be better to change the > behavior of platform_device_register() itself to call *_put() at the > error case internally. Or, if we keep the current behavior, at least > we should properly document it. > > Greg, Rafael, Danilo, what do you think? > > > thanks, > > Takashi I agree with your observation. Given that many callers of platform_device_register() do not handle the error path with a matching platform_device_put(), this does seem more like an issue related to the core/API behavior, or at least something that should be documented more clearly. Also, the underlying issue here appears to be related to the platform_device_register() core/API behavior. We are currently discussing in another similar case whether the better fix, if any, should be made in the core/API code rather than in individual callers: https://patchew.org/linux/20260415174159.3625777-1-lgs201920130244@gmail.com/ Once that discussion reaches a conclusion, we will revisit this and make the appropriate fix if needed. Thanks, Guangshuo ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ALSA: pcmtest: fix reference leak on failed device registration 2026-04-17 10:52 ` [PATCH] ALSA: pcmtest: fix reference leak on failed device registration Takashi Iwai 2026-04-17 11:30 ` Guangshuo Li @ 2026-04-23 5:19 ` Jiri Slaby 2026-04-23 7:39 ` Takashi Iwai 1 sibling, 1 reply; 4+ messages in thread From: Jiri Slaby @ 2026-04-23 5:19 UTC (permalink / raw) To: Takashi Iwai, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich Cc: Guangshuo Li, linux-kernel, driver-core On 17. 04. 26, 12:52, Takashi Iwai wrote: > On Fri, 17 Apr 2026 09:57:20 +0200, > Takashi Iwai wrote: >> >> On Wed, 15 Apr 2026 21:31:38 +0200, >> Guangshuo Li wrote: >>> >>> When platform_device_register() fails in mod_init(), the embedded struct >>> device in pcmtst_pdev has already been initialized by >>> device_initialize(), but the failure path returns the error without >>> dropping the device reference for the current platform device: >>> >>> mod_init() >>> -> platform_device_register(&pcmtst_pdev) >>> -> device_initialize(&pcmtst_pdev.dev) >>> -> setup_pdev_dma_masks(&pcmtst_pdev) >>> -> platform_device_add(&pcmtst_pdev) >>> >>> This leads to a reference leak when platform_device_register() fails. >>> Fix this by calling platform_device_put() before returning the error. >>> >>> The issue was identified by a static analysis tool I developed and >>> confirmed by manual review. >>> >>> Fixes: 315a3d57c64c5 ("ALSA: Implement the new Virtual PCM Test Driver") >>> Cc: stable@vger.kernel.org >>> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com> >> >> Thanks, applied now. > > ... and now I looked through the whole tree, and noticed that the > majority of callers of platform_device_register() don't care the error > cases without calling platform_device_put(). There are over a hundred > callers of platform_device_register() while only 5 or so are doing the > proper cleanup at the error. Moreover, unless the static pcmtst_pdev has ->type->release or ->release set, the patch triggers a warning upon put(). > Judging from the numbers above, it might be better to change the > behavior of platform_device_register() itself to call *_put() at the > error case internally. Yes, that's what should be done instead. thanks, -- js suse labs ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ALSA: pcmtest: fix reference leak on failed device registration 2026-04-23 5:19 ` Jiri Slaby @ 2026-04-23 7:39 ` Takashi Iwai 0 siblings, 0 replies; 4+ messages in thread From: Takashi Iwai @ 2026-04-23 7:39 UTC (permalink / raw) To: Jiri Slaby Cc: Takashi Iwai, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Guangshuo Li, linux-kernel, driver-core On Thu, 23 Apr 2026 07:19:20 +0200, Jiri Slaby wrote: > > On 17. 04. 26, 12:52, Takashi Iwai wrote: > > On Fri, 17 Apr 2026 09:57:20 +0200, > > Takashi Iwai wrote: > >> > >> On Wed, 15 Apr 2026 21:31:38 +0200, > >> Guangshuo Li wrote: > >>> > >>> When platform_device_register() fails in mod_init(), the embedded struct > >>> device in pcmtst_pdev has already been initialized by > >>> device_initialize(), but the failure path returns the error without > >>> dropping the device reference for the current platform device: > >>> > >>> mod_init() > >>> -> platform_device_register(&pcmtst_pdev) > >>> -> device_initialize(&pcmtst_pdev.dev) > >>> -> setup_pdev_dma_masks(&pcmtst_pdev) > >>> -> platform_device_add(&pcmtst_pdev) > >>> > >>> This leads to a reference leak when platform_device_register() fails. > >>> Fix this by calling platform_device_put() before returning the error. > >>> > >>> The issue was identified by a static analysis tool I developed and > >>> confirmed by manual review. > >>> > >>> Fixes: 315a3d57c64c5 ("ALSA: Implement the new Virtual PCM Test Driver") > >>> Cc: stable@vger.kernel.org > >>> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com> > >> > >> Thanks, applied now. > > > > ... and now I looked through the whole tree, and noticed that the > > majority of callers of platform_device_register() don't care the error > > cases without calling platform_device_put(). There are over a hundred > > callers of platform_device_register() while only 5 or so are doing the > > proper cleanup at the error. > > Moreover, unless the static pcmtst_pdev has ->type->release or > ->release set, the patch triggers a warning upon put(). Ah a good point, the call should be conditional. > > Judging from the numbers above, it might be better to change the > > behavior of platform_device_register() itself to call *_put() at the > > error case internally. > > Yes, that's what should be done instead. OK, I'm going to revert this patch for pcmtest.c for now (so back to the upstream state again), so that we can work on the platform_device_register() side. thanks, Takashi ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-04-23 7:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260415193138.3861297-1-lgs201920130244@gmail.com>
[not found] ` <87h5padnof.wl-tiwai@suse.de>
2026-04-17 10:52 ` [PATCH] ALSA: pcmtest: fix reference leak on failed device registration Takashi Iwai
2026-04-17 11:30 ` Guangshuo Li
2026-04-23 5:19 ` Jiri Slaby
2026-04-23 7:39 ` Takashi Iwai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox