* [PATCH 0/2] media: i2c: Fix some memleaks
@ 2022-12-08 7:59 Shang XiaoJing
2022-12-08 7:59 ` [PATCH 1/2] media: ov2740: Fix memleak in ov2740_init_controls() Shang XiaoJing
2022-12-08 7:59 ` [PATCH 2/2] media: ov5675: Fix memleak in ov5675_init_controls() Shang XiaoJing
0 siblings, 2 replies; 4+ messages in thread
From: Shang XiaoJing @ 2022-12-08 7:59 UTC (permalink / raw)
To: tian.shu.qiu, shawnx.tu, bingbu.cao, mchehab, sakari.ailus, linux-media
Cc: shangxiaojing
Some memleaks are found due to the missing v4l2_ctrl_handler_free() and
fixed by this patch.
Shang XiaoJing (2):
media: ov2740: Fix memleak in ov2740_init_controls()
media: ov5675: Fix memleak in ov5675_init_controls()
drivers/media/i2c/ov2740.c | 4 +++-
drivers/media/i2c/ov5675.c | 4 +++-
2 files changed, 6 insertions(+), 2 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] media: ov2740: Fix memleak in ov2740_init_controls()
2022-12-08 7:59 [PATCH 0/2] media: i2c: Fix some memleaks Shang XiaoJing
@ 2022-12-08 7:59 ` Shang XiaoJing
2023-03-21 12:32 ` Dave Stevenson
2022-12-08 7:59 ` [PATCH 2/2] media: ov5675: Fix memleak in ov5675_init_controls() Shang XiaoJing
1 sibling, 1 reply; 4+ messages in thread
From: Shang XiaoJing @ 2022-12-08 7:59 UTC (permalink / raw)
To: tian.shu.qiu, shawnx.tu, bingbu.cao, mchehab, sakari.ailus, linux-media
Cc: shangxiaojing
There is a kmemleak when testing the media/i2c/ov2740.c with bpf mock
device:
unreferenced object 0xffff8881090e19e0 (size 16):
comm "51-i2c-ov2740", pid 278, jiffies 4294781584 (age 23.613s)
hex dump (first 16 bytes):
00 f3 7c 0b 81 88 ff ff 80 75 6a 09 81 88 ff ff ..|......uj.....
backtrace:
[<000000004e9fad8f>] __kmalloc_node+0x44/0x1b0
[<0000000039c802f4>] kvmalloc_node+0x34/0x180
[<000000009b8b5c63>] v4l2_ctrl_handler_init_class+0x11d/0x180
[videodev]
[<0000000038644056>] ov2740_probe+0x37d/0x84f [ov2740]
[<0000000092489f59>] i2c_device_probe+0x28d/0x680
[<000000001038babe>] really_probe+0x17c/0x3f0
[<0000000098c7af1c>] __driver_probe_device+0xe3/0x170
[<00000000e1b3dc24>] device_driver_attach+0x34/0x80
[<000000005a04a34d>] bind_store+0x10b/0x1a0
[<00000000ce25d4f2>] drv_attr_store+0x49/0x70
[<000000007d9f4e9a>] sysfs_kf_write+0x8c/0xb0
[<00000000be6cff0f>] kernfs_fop_write_iter+0x216/0x2e0
[<0000000031ddb40a>] vfs_write+0x658/0x810
[<0000000041beecdd>] ksys_write+0xd6/0x1b0
[<0000000023755840>] do_syscall_64+0x38/0x90
[<00000000b2cc2da2>] entry_SYSCALL_64_after_hwframe+0x63/0xcd
ov2740_init_controls() won't clean all the allocated resources in fail
path, which may causes the memleaks. Add v4l2_ctrl_handler_free() to
prevent memleak.
Fixes: 866edc895171 ("media: i2c: Add ov2740 image sensor driver")
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
drivers/media/i2c/ov2740.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
index 5d74ad479214..628ab86698c0 100644
--- a/drivers/media/i2c/ov2740.c
+++ b/drivers/media/i2c/ov2740.c
@@ -630,8 +630,10 @@ static int ov2740_init_controls(struct ov2740 *ov2740)
V4L2_CID_TEST_PATTERN,
ARRAY_SIZE(ov2740_test_pattern_menu) - 1,
0, 0, ov2740_test_pattern_menu);
- if (ctrl_hdlr->error)
+ if (ctrl_hdlr->error) {
+ v4l2_ctrl_handler_free(ctrl_hdlr);
return ctrl_hdlr->error;
+ }
ov2740->sd.ctrl_handler = ctrl_hdlr;
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] media: ov5675: Fix memleak in ov5675_init_controls()
2022-12-08 7:59 [PATCH 0/2] media: i2c: Fix some memleaks Shang XiaoJing
2022-12-08 7:59 ` [PATCH 1/2] media: ov2740: Fix memleak in ov2740_init_controls() Shang XiaoJing
@ 2022-12-08 7:59 ` Shang XiaoJing
1 sibling, 0 replies; 4+ messages in thread
From: Shang XiaoJing @ 2022-12-08 7:59 UTC (permalink / raw)
To: tian.shu.qiu, shawnx.tu, bingbu.cao, mchehab, sakari.ailus, linux-media
Cc: shangxiaojing
There is a kmemleak when testing the media/i2c/ov5675.c with bpf mock
device:
AssertionError: unreferenced object 0xffff888107362160 (size 16):
comm "python3", pid 277, jiffies 4294832798 (age 20.722s)
hex dump (first 16 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000abe7d67c>] __kmalloc_node+0x44/0x1b0
[<000000008a725aac>] kvmalloc_node+0x34/0x180
[<000000009a53cd11>] v4l2_ctrl_handler_init_class+0x11d/0x180
[videodev]
[<0000000055b46db0>] ov5675_probe+0x38b/0x897 [ov5675]
[<00000000153d886c>] i2c_device_probe+0x28d/0x680
[<000000004afb7e8f>] really_probe+0x17c/0x3f0
[<00000000ff2f18e4>] __driver_probe_device+0xe3/0x170
[<000000000a001029>] driver_probe_device+0x49/0x120
[<00000000e39743c7>] __device_attach_driver+0xf7/0x150
[<00000000d32fd070>] bus_for_each_drv+0x114/0x180
[<000000009083ac41>] __device_attach+0x1e5/0x2d0
[<0000000015b4a830>] bus_probe_device+0x126/0x140
[<000000007813deaf>] device_add+0x810/0x1130
[<000000007becb867>] i2c_new_client_device+0x386/0x540
[<000000007f9cf4b4>] of_i2c_register_device+0xf1/0x110
[<00000000ebfdd032>] of_i2c_notify+0xfc/0x1f0
ov5675_init_controls() won't clean all the allocated resources in fail
path, which may causes the memleaks. Add v4l2_ctrl_handler_free() to
prevent memleak.
Fixes: bf27502b1f3b ("media: ov5675: Add support for OV5675 sensor")
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
drivers/media/i2c/ov5675.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov5675.c b/drivers/media/i2c/ov5675.c
index 94dc8cb7a7c0..a6e6b367d128 100644
--- a/drivers/media/i2c/ov5675.c
+++ b/drivers/media/i2c/ov5675.c
@@ -820,8 +820,10 @@ static int ov5675_init_controls(struct ov5675 *ov5675)
v4l2_ctrl_new_std(ctrl_hdlr, &ov5675_ctrl_ops,
V4L2_CID_VFLIP, 0, 1, 1, 0);
- if (ctrl_hdlr->error)
+ if (ctrl_hdlr->error) {
+ v4l2_ctrl_handler_free(ctrl_hdlr);
return ctrl_hdlr->error;
+ }
ov5675->sd.ctrl_handler = ctrl_hdlr;
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] media: ov2740: Fix memleak in ov2740_init_controls()
2022-12-08 7:59 ` [PATCH 1/2] media: ov2740: Fix memleak in ov2740_init_controls() Shang XiaoJing
@ 2023-03-21 12:32 ` Dave Stevenson
0 siblings, 0 replies; 4+ messages in thread
From: Dave Stevenson @ 2023-03-21 12:32 UTC (permalink / raw)
To: Shang XiaoJing
Cc: tian.shu.qiu, shawnx.tu, bingbu.cao, mchehab, sakari.ailus,
linux-media, Hans Verkuil
Hi
On Thu, 8 Dec 2022 at 08:01, Shang XiaoJing <shangxiaojing@huawei.com> wrote:
>
> There is a kmemleak when testing the media/i2c/ov2740.c with bpf mock
> device:
>
> unreferenced object 0xffff8881090e19e0 (size 16):
> comm "51-i2c-ov2740", pid 278, jiffies 4294781584 (age 23.613s)
> hex dump (first 16 bytes):
> 00 f3 7c 0b 81 88 ff ff 80 75 6a 09 81 88 ff ff ..|......uj.....
> backtrace:
> [<000000004e9fad8f>] __kmalloc_node+0x44/0x1b0
> [<0000000039c802f4>] kvmalloc_node+0x34/0x180
> [<000000009b8b5c63>] v4l2_ctrl_handler_init_class+0x11d/0x180
> [videodev]
> [<0000000038644056>] ov2740_probe+0x37d/0x84f [ov2740]
> [<0000000092489f59>] i2c_device_probe+0x28d/0x680
> [<000000001038babe>] really_probe+0x17c/0x3f0
> [<0000000098c7af1c>] __driver_probe_device+0xe3/0x170
> [<00000000e1b3dc24>] device_driver_attach+0x34/0x80
> [<000000005a04a34d>] bind_store+0x10b/0x1a0
> [<00000000ce25d4f2>] drv_attr_store+0x49/0x70
> [<000000007d9f4e9a>] sysfs_kf_write+0x8c/0xb0
> [<00000000be6cff0f>] kernfs_fop_write_iter+0x216/0x2e0
> [<0000000031ddb40a>] vfs_write+0x658/0x810
> [<0000000041beecdd>] ksys_write+0xd6/0x1b0
> [<0000000023755840>] do_syscall_64+0x38/0x90
> [<00000000b2cc2da2>] entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
> ov2740_init_controls() won't clean all the allocated resources in fail
> path, which may causes the memleaks. Add v4l2_ctrl_handler_free() to
> prevent memleak.
>
> Fixes: 866edc895171 ("media: i2c: Add ov2740 image sensor driver")
> Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
> ---
> drivers/media/i2c/ov2740.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
> index 5d74ad479214..628ab86698c0 100644
> --- a/drivers/media/i2c/ov2740.c
> +++ b/drivers/media/i2c/ov2740.c
> @@ -630,8 +630,10 @@ static int ov2740_init_controls(struct ov2740 *ov2740)
> V4L2_CID_TEST_PATTERN,
> ARRAY_SIZE(ov2740_test_pattern_menu) - 1,
> 0, 0, ov2740_test_pattern_menu);
> - if (ctrl_hdlr->error)
> + if (ctrl_hdlr->error) {
> + v4l2_ctrl_handler_free(ctrl_hdlr);
> return ctrl_hdlr->error;
I know this has been merged, but I happened to be looking at ov2740
for other reasons and noted this patch.
v4l2_ctrl_handler_free includes setting "hdl->error = 0;" [1], so as I
read it, calling it here means that ov2740_init_controls will now be
returning 0 rather than the error code.
There is a v4l2_ctrl_handler_free call in ov2740_probe, but it's
freeing ov2740->sd.ctrl_handler which is only set by
ov2740_init_controls on success. Perhaps it's better to call it with
&ov2740->ctrl_handler instead?
The same issue applies to the other patch in this series for ov5675 [3].
Doing a very quick sweep through the tree, it looks like at least
imx296, imx334, imx335, imx412, ov7251, and ov9282 all appear to have
the same issue.
As it is a useful pattern, is it better to NOT clear hdl->error in
v4l2_ctrl_handler_free? It'll be set to 0 in any subsequent call to
v4l2_ctrl_handler_init, so leaving it set has no real adverse effect.
Sakari & Hans - your thoughts?
Dave
[1] https://elixir.bootlin.com/linux/latest/source/drivers/media/v4l2-core/v4l2-ctrls-core.c#L1330
[2] https://elixir.bootlin.com/linux/latest/source/drivers/media/i2c/ov2740.c#L1196
[3] https://github.com/torvalds/linux/commit/dd74ed6c213003533e3abf4c204374ef01d86978
> + }
>
> ov2740->sd.ctrl_handler = ctrl_hdlr;
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-03-21 12:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-08 7:59 [PATCH 0/2] media: i2c: Fix some memleaks Shang XiaoJing
2022-12-08 7:59 ` [PATCH 1/2] media: ov2740: Fix memleak in ov2740_init_controls() Shang XiaoJing
2023-03-21 12:32 ` Dave Stevenson
2022-12-08 7:59 ` [PATCH 2/2] media: ov5675: Fix memleak in ov5675_init_controls() Shang XiaoJing
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.