Skip to contentSkip to conversation

: media : hackrf : memory leak

PATCH In review Yavuz, Tuba 2 participants 3 messages Jan 27, 2019 – Jan 28, 2019 Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 : media : hackrf : memory leak

Due to a missing v4l2_device_get function in the hackrf_probe function, the reference count of the v4l2_device object reaches zero inside the wrong API function (video_unregister_device) instead of v4l2_device_put. This causes a memory leak as the release callback would not get called.

---

media/usb/hackrf/hackrf.c renamed from media/usb/hackrf/hackrf.c.orig +0 -0
@@ -1524,6 +1524,7 @@ static int hackrf_probe(struct usb_inter

  "Failed to register as video device (%d)\n", ret); goto err_video_unregister_device_rx;
  }
+ v4l2_device_get(&dev->v4l2_dev);
  dev_info(dev->dev, "Registered as %s\n", video_device_node_name(&dev->tx_vdev));​

#1 lore raw

Hi Tuba,

Quoted 6 lines · Tuba

Due to a missing v4l2_device_get function in the hackrf_probe function, the reference count of the v4l2_device object reaches zero inside the wrong API function (video_unregister_device) instead of v4l2_device_put. This causes a memory leak as the release callback would not get called.

The refcount is 1 after calling v4l2_device_register(). Each video_register_device call will increment the refcount by 1. When a video node is released (v4l2_device_release) the refcount is decremented, and when the device is disconnected (hackrf_disconnect) the refcount is decremented again.

So I don't see where there is a memory leak, and neither do I understand how incrementing the refcount would prevent a memory leak. I would expect that it causes a memory leak!

Is there something else going on here?

Regards,

Hans

Quoted 18 lines

---

media/usb/hackrf/hackrf.c renamed from media/usb/hackrf/hackrf.c.orig +1 -0
@@ -1524,6 +1524,7 @@ static int hackrf_probe(struct usb_inter
  			"Failed to register as video device (%d)\n", ret);
  		goto err_video_unregister_device_rx;
  	}
+	v4l2_device_get(&dev->v4l2_dev);
  	dev_info(dev->dev, "Registered as %s\n",

  video_device_node_name(&dev->tx_vdev));​

#2 lore raw

Hi Hans,

You are right..

This was a false positive due to a bug in my environment model. Obviously, v4l2_device_disconnect does not affect the v4l2_device reference count.

Best,

Tuba
________________________________________
From: Hans Verkuil <[email protected]>
Sent: Monday, January 28, 2019 9:10 AM
To: Yavuz, Tuba; [email protected]
Cc: Greg KH
Subject: Re: [PATCH] : media : hackrf : memory leak

Hi Tuba,

Quoted 6 lines · Tuba

Due to a missing v4l2_device_get function in the hackrf_probe function, the reference count of the v4l2_device object reaches zero inside the wrong API function (video_unregister_device) instead of v4l2_device_put. This causes a memory leak as the release callback would not get called.

The refcount is 1 after calling v4l2_device_register(). Each video_register_device call will increment the refcount by 1. When a video node is released (v4l2_device_release) the refcount is decremented, and when the device is disconnected (hackrf_disconnect) the refcount is decremented again.

So I don't see where there is a memory leak, and neither do I understand how incrementing the refcount would prevent a memory leak. I would expect that it causes a memory leak!

Is there something else going on here?

Regards,

Hans

Quoted 18 lines

---

media/usb/hackrf/hackrf.c 2019-01-27 17:50:41.660736688 -0500 renamed from media/usb/hackrf/hackrf.c.orig 2019-01-26 11:37:18.912210823 -0500 +1 -0
@@ -1524,6 +1524,7 @@ static int hackrf_probe(struct usb_inter
                      "Failed to register as video device (%d)\n", ret);
              goto err_video_unregister_device_rx;
      }
+     v4l2_device_get(&dev->v4l2_dev);
      dev_info(dev->dev, "Registered as %s\n",
               video_device_node_name(&dev->tx_vdev));​