When cdev_device_add() failed, calling put_device() to explicitly release dev->lirc_dev. Otherwise, it could cause the fault of the reference count.
Found by code review.
Cc: stable@vger.kernel.org Fixes: a6ddd4fecbb0 ("media: lirc: remove last remnants of lirc kapi") Signed-off-by: Ma Ke make24@iscas.ac.cn --- drivers/media/rc/lirc_dev.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index a2257dc2f25d..ed839e15fa16 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -765,6 +765,7 @@ int lirc_register(struct rc_dev *dev) return 0;
out_ida: + put_device(&dev->lirc_dev); ida_free(&lirc_ida, minor); return err; }
Hi,
On Sun, Jan 05, 2025 at 06:01:01PM +0800, Ma Ke wrote:
When cdev_device_add() failed, calling put_device() to explicitly release dev->lirc_dev. Otherwise, it could cause the fault of the reference count.
Found by code review.
Interesting find, thanks for finding and reporting.
So I think the idea is right, but there is a problem. lirc_release_device() will do a put_device() on the rcdev, but no corresponding get_device() is done in this code path.
Sean
Cc: stable@vger.kernel.org Fixes: a6ddd4fecbb0 ("media: lirc: remove last remnants of lirc kapi") Signed-off-by: Ma Ke make24@iscas.ac.cn
drivers/media/rc/lirc_dev.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index a2257dc2f25d..ed839e15fa16 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -765,6 +765,7 @@ int lirc_register(struct rc_dev *dev) return 0; out_ida:
- put_device(&dev->lirc_dev); ida_free(&lirc_ida, minor); return err;
}
2.25.1
Sean Youngsean@mess.org wrote:
Hi,
On Sun, Jan 05, 2025 at 06:01:01PM +0800, Ma Ke wrote:
When cdev_device_add() failed, calling put_device() to explicitly release dev->lirc_dev. Otherwise, it could cause the fault of the reference count.
Found by code review.
Interesting find, thanks for finding and reporting.
So I think the idea is right, but there is a problem. lirc_release_device() will do a put_device() on the rcdev, but no corresponding get_device() is done in this code path.
Sean
Thank you for your reply and suggestions. Following your instructions, I took a close look at the code. Perhaps you meant to suggest removing the put_device() call from lirc_release_device(), effectively making lirc_release_device() an empty function? Looking forward to your reply. -- Regards,
Ma Ke
On Tue, Jan 07, 2025 at 09:51:43AM +0800, Ma Ke wrote:
Sean Youngsean@mess.org wrote:
Hi,
On Sun, Jan 05, 2025 at 06:01:01PM +0800, Ma Ke wrote:
When cdev_device_add() failed, calling put_device() to explicitly release dev->lirc_dev. Otherwise, it could cause the fault of the reference count.
Found by code review.
Interesting find, thanks for finding and reporting.
So I think the idea is right, but there is a problem. lirc_release_device() will do a put_device() on the rcdev, but no corresponding get_device() is done in this code path.
Sean
Thank you for your reply and suggestions. Following your instructions, I took a close look at the code. Perhaps you meant to suggest removing the put_device() call from lirc_release_device(), effectively making lirc_release_device() an empty function?
That would introduce a memory leak and presumably the rc device would never be cleaned up, so no I don't think that would work.
I'm not sure what the right solution is yet.
Sean
linux-stable-mirror@lists.linaro.org
-
Ma Ke -
Sean Young