linux-staging.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] most: core: fix resource leak in most_register_interface error paths
@ 2025-11-25 22:56 Navaneeth K
  2025-11-25 22:56 ` [PATCH v2 1/2] " Navaneeth K
  2025-11-25 22:56 ` [PATCH v2 2/2] most: usb: remove double cleanup of interface on registration failure Navaneeth K
  0 siblings, 2 replies; 7+ messages in thread
From: Navaneeth K @ 2025-11-25 22:56 UTC (permalink / raw)
  To: parthiban.veerasooran, christian.gromm, gregkh
  Cc: linux-staging, linux-kernel, Navaneeth K

This series fixes a resource leak in most_register_interface() where
early errors (allocation failures) would return without releasing the
device, leaking memory.

It switches to the split device_initialize() + device_add() pattern
to ensure put_device() can be safely called on all error paths.

It also updates the most_usb driver to remove manual error handling
that would otherwise cause a double-free with the new core logic.

Changes in v2:
  - Replaced the previous single-driver fix ("staging: most: dim2: fix
    missing cleanup...") which was NACKed because it introduced a
    double-free risk.
  - Moved the fix to the Core (core.c) to handle cleanup centrally via
    put_device().
  - Updated most_usb.c to remove conflicting manual cleanup.
  - Dropped the dim2.c patch as the upstream code is already correct for
    this new core logic (it relies on the release callback, which core
    now triggers).

Navaneeth K (2):
  most: core: fix resource leak in most_register_interface error paths
  most: usb: remove double cleanup of interface on registration failure

 drivers/most/core.c     |  9 +++++++--
 drivers/most/most_usb.c | 11 +++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 1/2] most: core: fix resource leak in most_register_interface error paths
  2025-11-25 22:56 [PATCH v2 0/2] most: core: fix resource leak in most_register_interface error paths Navaneeth K
@ 2025-11-25 22:56 ` Navaneeth K
  2025-11-25 23:10   ` Navaneeth K
  2025-11-26 17:23   ` Abdun Nihaal
  2025-11-25 22:56 ` [PATCH v2 2/2] most: usb: remove double cleanup of interface on registration failure Navaneeth K
  1 sibling, 2 replies; 7+ messages in thread
From: Navaneeth K @ 2025-11-25 22:56 UTC (permalink / raw)
  To: parthiban.veerasooran, christian.gromm, gregkh
  Cc: linux-staging, linux-kernel, Navaneeth K

The function most_register_interface() did not correctly release resources
if it failed early (before device_register). In these cases, it returned
an error code immediately, leaking the memory allocated for the interface.

Fix this by initializing the device early via device_initialize() and
calling put_device() on all error paths. This ensures the release
callback is triggered to free memory.

Switch to using device_add() instead of device_register() to handle
the split initialization.

Signed-off-by: Navaneeth K <knavaneeth786@gmail.com>
---
 drivers/most/core.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/most/core.c b/drivers/most/core.c
index da319d108ea1d..8635fd08035e9 100644
--- a/drivers/most/core.c
+++ b/drivers/most/core.c
@@ -1283,18 +1283,23 @@ int most_register_interface(struct most_interface *iface)
 	struct most_channel *c;
 
 	if (!iface || !iface->enqueue || !iface->configure ||
-	    !iface->poison_channel || (iface->num_channels > MAX_CHANNELS))
+	    !iface->poison_channel || (iface->num_channels > MAX_CHANNELS) ||
+	    !iface->dev)
 		return -EINVAL;
 
+	device_initialize(iface->dev);
+
 	id = ida_alloc(&mdev_id, GFP_KERNEL);
 	if (id < 0) {
 		dev_err(iface->dev, "Failed to allocate device ID\n");
+		put_device(iface->dev);
 		return id;
 	}
 
 	iface->p = kzalloc(sizeof(*iface->p), GFP_KERNEL);
 	if (!iface->p) {
 		ida_free(&mdev_id, id);
+		put_device(iface->dev);
 		return -ENOMEM;
 	}
 
@@ -1304,7 +1309,7 @@ int most_register_interface(struct most_interface *iface)
 	iface->dev->bus = &mostbus;
 	iface->dev->groups = interface_attr_groups;
 	dev_set_drvdata(iface->dev, iface);
-	if (device_register(iface->dev)) {
+	if (device_add(iface->dev)) {
 		dev_err(iface->dev, "Failed to register interface device\n");
 		kfree(iface->p);
 		put_device(iface->dev);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/2] most: usb: remove double cleanup of interface on registration failure
  2025-11-25 22:56 [PATCH v2 0/2] most: core: fix resource leak in most_register_interface error paths Navaneeth K
  2025-11-25 22:56 ` [PATCH v2 1/2] " Navaneeth K
@ 2025-11-25 22:56 ` Navaneeth K
  2025-11-26 17:31   ` Abdun Nihaal
  1 sibling, 1 reply; 7+ messages in thread
From: Navaneeth K @ 2025-11-25 22:56 UTC (permalink / raw)
  To: parthiban.veerasooran, christian.gromm, gregkh
  Cc: linux-staging, linux-kernel, Navaneeth K

Since most_register_interface() now correctly handles cleanup (calling
put_device on failure), the manual cleanup in hdm_probe() would cause
a double-free.

Remove the manual cleanup labels and return the error code directly.

Signed-off-by: Navaneeth K <knavaneeth786@gmail.com>
---
 drivers/most/most_usb.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/most/most_usb.c b/drivers/most/most_usb.c
index 10064d7b72498..597cf7f4e6b90 100644
--- a/drivers/most/most_usb.c
+++ b/drivers/most/most_usb.c
@@ -1058,7 +1058,7 @@ hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
 
 	ret = most_register_interface(&mdev->iface);
 	if (ret)
-		goto err_free_busy_urbs;
+		return ret;
 
 	mutex_lock(&mdev->io_mutex);
 	if (le16_to_cpu(usb_dev->descriptor.idProduct) == USB_DEV_ID_OS81118 ||
@@ -1068,8 +1068,7 @@ hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
 		if (!mdev->dci) {
 			mutex_unlock(&mdev->io_mutex);
 			most_deregister_interface(&mdev->iface);
-			ret = -ENOMEM;
-			goto err_free_busy_urbs;
+			return -ENOMEM;
 		}
 
 		mdev->dci->dev.init_name = "dci";
@@ -1077,19 +1076,15 @@ hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
 		mdev->dci->dev.groups = dci_groups;
 		mdev->dci->dev.release = release_dci;
 		if (device_register(&mdev->dci->dev)) {
+			put_device(&mdev->dci->dev);
 			mutex_unlock(&mdev->io_mutex);
 			most_deregister_interface(&mdev->iface);
-			ret = -ENOMEM;
-			goto err_free_dci;
+			return -ENOMEM;
 		}
 		mdev->dci->usb_device = mdev->usb_device;
 	}
 	mutex_unlock(&mdev->io_mutex);
 	return 0;
-err_free_dci:
-	put_device(&mdev->dci->dev);
-err_free_busy_urbs:
-	kfree(mdev->busy_urbs);
 err_free_ep_address:
 	kfree(mdev->ep_address);
 err_free_cap:
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] most: core: fix resource leak in most_register_interface error paths
  2025-11-25 22:56 ` [PATCH v2 1/2] " Navaneeth K
@ 2025-11-25 23:10   ` Navaneeth K
  2025-11-26 17:23   ` Abdun Nihaal
  1 sibling, 0 replies; 7+ messages in thread
From: Navaneeth K @ 2025-11-25 23:10 UTC (permalink / raw)
  To: parthiban.veerasooran, christian.gromm, gregkh
  Cc: linux-staging, linux-kernel

Apologies, I missed adding the credit for the original finding.
If a v3 is needed for other reasons, I will include it. Otherwise, if 
this is merged, please add:
Reported-by: Abdun Nihaal <abdun.nihaal@gmail.com>


On 26-11-2025 04:26, Navaneeth K wrote:
> The function most_register_interface() did not correctly release resources
> if it failed early (before device_register). In these cases, it returned
> an error code immediately, leaking the memory allocated for the interface.
>
> Fix this by initializing the device early via device_initialize() and
> calling put_device() on all error paths. This ensures the release
> callback is triggered to free memory.
>
> Switch to using device_add() instead of device_register() to handle
> the split initialization.
>
> Signed-off-by: Navaneeth K <knavaneeth786@gmail.com>
> ---
>   drivers/most/core.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/most/core.c b/drivers/most/core.c
> index da319d108ea1d..8635fd08035e9 100644
> --- a/drivers/most/core.c
> +++ b/drivers/most/core.c
> @@ -1283,18 +1283,23 @@ int most_register_interface(struct most_interface *iface)
>   	struct most_channel *c;
>   
>   	if (!iface || !iface->enqueue || !iface->configure ||
> -	    !iface->poison_channel || (iface->num_channels > MAX_CHANNELS))
> +	    !iface->poison_channel || (iface->num_channels > MAX_CHANNELS) ||
> +	    !iface->dev)
>   		return -EINVAL;
>   
> +	device_initialize(iface->dev);
> +
>   	id = ida_alloc(&mdev_id, GFP_KERNEL);
>   	if (id < 0) {
>   		dev_err(iface->dev, "Failed to allocate device ID\n");
> +		put_device(iface->dev);
>   		return id;
>   	}
>   
>   	iface->p = kzalloc(sizeof(*iface->p), GFP_KERNEL);
>   	if (!iface->p) {
>   		ida_free(&mdev_id, id);
> +		put_device(iface->dev);
>   		return -ENOMEM;
>   	}
>   
> @@ -1304,7 +1309,7 @@ int most_register_interface(struct most_interface *iface)
>   	iface->dev->bus = &mostbus;
>   	iface->dev->groups = interface_attr_groups;
>   	dev_set_drvdata(iface->dev, iface);
> -	if (device_register(iface->dev)) {
> +	if (device_add(iface->dev)) {
>   		dev_err(iface->dev, "Failed to register interface device\n");
>   		kfree(iface->p);
>   		put_device(iface->dev);

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] most: core: fix resource leak in most_register_interface error paths
  2025-11-25 22:56 ` [PATCH v2 1/2] " Navaneeth K
  2025-11-25 23:10   ` Navaneeth K
@ 2025-11-26 17:23   ` Abdun Nihaal
  1 sibling, 0 replies; 7+ messages in thread
From: Abdun Nihaal @ 2025-11-26 17:23 UTC (permalink / raw)
  To: Navaneeth K
  Cc: parthiban.veerasooran, christian.gromm, gregkh, linux-staging,
	linux-kernel

On Tue, Nov 25, 2025 at 10:56:05PM +0000, Navaneeth K wrote:
> The function most_register_interface() did not correctly release resources
> if it failed early (before device_register). In these cases, it returned
> an error code immediately, leaking the memory allocated for the interface.
> 
> Fix this by initializing the device early via device_initialize() and
> calling put_device() on all error paths. This ensures the release
> callback is triggered to free memory.
> 
> Switch to using device_add() instead of device_register() to handle
> the split initialization.

Acked-by: Abdun Nihaal <abdun.nihaal@gmail.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] most: usb: remove double cleanup of interface on registration failure
  2025-11-25 22:56 ` [PATCH v2 2/2] most: usb: remove double cleanup of interface on registration failure Navaneeth K
@ 2025-11-26 17:31   ` Abdun Nihaal
  2025-11-26 22:08     ` Navaneeth K
  0 siblings, 1 reply; 7+ messages in thread
From: Abdun Nihaal @ 2025-11-26 17:31 UTC (permalink / raw)
  To: Navaneeth K
  Cc: parthiban.veerasooran, christian.gromm, gregkh, linux-staging,
	linux-kernel

On Tue, Nov 25, 2025 at 10:56:06PM +0000, Navaneeth K wrote:
> Since most_register_interface() now correctly handles cleanup (calling
> put_device on failure), the manual cleanup in hdm_probe() would cause
> a double-free.
> 
> Remove the manual cleanup labels and return the error code directly.

This exact change has already been done by Johan Hovold in the following
patch: https://lore.kernel.org/all/20251029093029.28922-1-johan@kernel.org/

I find that patch in Linux-next. It must have been already applied.
and so you can drop this patch.

Regards,
Nihaal

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] most: usb: remove double cleanup of interface on registration failure
  2025-11-26 17:31   ` Abdun Nihaal
@ 2025-11-26 22:08     ` Navaneeth K
  0 siblings, 0 replies; 7+ messages in thread
From: Navaneeth K @ 2025-11-26 22:08 UTC (permalink / raw)
  To: Abdun Nihaal
  Cc: parthiban.veerasooran, christian.gromm, gregkh, linux-staging,
	linux-kernel

Hi Nihaal,

Thanks for pointing that out. I missed that Johan's patch was already in 
linux-next.

I will drop this patch and send v3 containing only the core fix (which 
is still needed to prevent the leak).

Regards,
Navaneeth


On 26-11-2025 23:01, Abdun Nihaal wrote:
> On Tue, Nov 25, 2025 at 10:56:06PM +0000, Navaneeth K wrote:
>> Since most_register_interface() now correctly handles cleanup (calling
>> put_device on failure), the manual cleanup in hdm_probe() would cause
>> a double-free.
>>
>> Remove the manual cleanup labels and return the error code directly.
> This exact change has already been done by Johan Hovold in the following
> patch: https://lore.kernel.org/all/20251029093029.28922-1-johan@kernel.org/
>
> I find that patch in Linux-next. It must have been already applied.
> and so you can drop this patch.
>
> Regards,
> Nihaal

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-11-26 22:08 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-25 22:56 [PATCH v2 0/2] most: core: fix resource leak in most_register_interface error paths Navaneeth K
2025-11-25 22:56 ` [PATCH v2 1/2] " Navaneeth K
2025-11-25 23:10   ` Navaneeth K
2025-11-26 17:23   ` Abdun Nihaal
2025-11-25 22:56 ` [PATCH v2 2/2] most: usb: remove double cleanup of interface on registration failure Navaneeth K
2025-11-26 17:31   ` Abdun Nihaal
2025-11-26 22:08     ` Navaneeth K

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).