2015-02-18 16:32:49

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH] [media] soc-camera: Remove bogus devm_kfree() in soc_of_bind()

Unlike scan_async_group(), soc_of_bind() doesn't allocate its
soc_camera_async_client structure using devm_kzalloc(), but has it
embedded inside the soc_of_info structure. Hence on failure, it must
not free it using devm_kfree(), as this will cause a warning, and may
cause slab corruption:

soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at drivers/base/devres.c:887 devm_kfree+0x30/0x40()
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.19.0-shmobile-08386-g37feb0d093cb2d8e #128
Hardware name: Generic R8A7791 (Flattened Device Tree)
Backtrace:
[<c0011e7c>] (dump_backtrace) from [<c0012024>] (show_stack+0x18/0x1c)
r6:c05a923b r5:00000009 r4:00000000 r3:00204140
[<c001200c>] (show_stack) from [<c048ed30>] (dump_stack+0x78/0x94)
[<c048ecb8>] (dump_stack) from [<c002687c>] (warn_slowpath_common+0x8c/0xb8)
r4:00000000 r3:00000000
[<c00267f0>] (warn_slowpath_common) from [<c0026980>] (warn_slowpath_null+0x24/0x2c)
r8:ee7d8214 r7:ed83b810 r6:ed83bc20 r5:fffffffa r4:ed83e510
[<c002695c>] (warn_slowpath_null) from [<c025e0cc>] (devm_kfree+0x30/0x40)
[<c025e09c>] (devm_kfree) from [<c032bbf4>] (soc_of_bind.isra.14+0x194/0x1d4)
[<c032ba60>] (soc_of_bind.isra.14) from [<c032c6b8>] (soc_camera_host_register+0x208/0x31c)
r9:00000070 r8:ee7e05d0 r7:ee153210 r6:00000000 r5:ee7e0218 r4:ed83bc20
[<c032c4b0>] (soc_camera_host_register) from [<c032e80c>] (rcar_vin_probe+0x1f4/0x238)
r8:ee153200 r7:00000008 r6:ee153210 r5:ed83bc10 r4:c066319c r3:000000c0
[<c032e618>] (rcar_vin_probe) from [<c025c334>] (platform_drv_probe+0x50/0xa0)
r10:00000000 r9:c0662fa8 r8:00000000 r7:c06a3700 r6:c0662fa8 r5:ee153210
r4:00000000
[<c025c2e4>] (platform_drv_probe) from [<c025af08>] (driver_probe_device+0xc4/0x208)
r6:c06a36f4 r5:00000000 r4:ee153210 r3:c025c2e4
[<c025ae44>] (driver_probe_device) from [<c025b108>] (__driver_attach+0x70/0x94)
r9:c066f9c0 r8:c0624a98 r7:c065b790 r6:c0662fa8 r5:ee153244 r4:ee153210
[<c025b098>] (__driver_attach) from [<c025984c>] (bus_for_each_dev+0x74/0x98)
r6:c025b098 r5:c0662fa8 r4:00000000 r3:00000001
[<c02597d8>] (bus_for_each_dev) from [<c025b1dc>] (driver_attach+0x20/0x28)
r6:ed83c200 r5:00000000 r4:c0662fa8
[<c025b1bc>] (driver_attach) from [<c025a00c>] (bus_add_driver+0xdc/0x1c4)
[<c0259f30>] (bus_add_driver) from [<c025b8f4>] (driver_register+0xa4/0xe8)
r7:c0624a98 r6:00000000 r5:c060b010 r4:c0662fa8
[<c025b850>] (driver_register) from [<c025ccd0>] (__platform_driver_register+0x50/0x64)
r5:c060b010 r4:ed8394c0
[<c025cc80>] (__platform_driver_register) from [<c060b028>] (rcar_vin_driver_init+0x18/0x20)
[<c060b010>] (rcar_vin_driver_init) from [<c05edde8>] (do_one_initcall+0x108/0x1b8)
[<c05edce0>] (do_one_initcall) from [<c05edfb4>] (kernel_init_freeable+0x11c/0x1e4)
r9:c066f9c0 r8:c066f9c0 r7:c062eab0 r6:c06252c4 r5:000000ad r4:00000006
[<c05ede98>] (kernel_init_freeable) from [<c048c3d0>] (kernel_init+0x10/0xec)
r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c048c3c0 r4:00000000
[<c048c3c0>] (kernel_init) from [<c000eba0>] (ret_from_fork+0x14/0x34)
r4:00000000 r3:ee04e000
---[ end trace e3a984cc0335c8a0 ]---
rcar_vin e6ef1000.video: group probe failed: -6

Fixes: 1ddc6a6caa94e1e1 ("[media] soc_camera: add support for dt binding soc_camera drivers")
Cc: [email protected]
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Triggered with shmobile-defconfig on r8a7791/koelsch.
---
drivers/media/platform/soc_camera/soc_camera.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
index cee7b56f84049944..d8a072fe46035821 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1665,7 +1665,6 @@ eclkreg:
eaddpdev:
platform_device_put(sasc->pdev);
eallocpdev:
- devm_kfree(ici->v4l2_dev.dev, sasc);
dev_err(ici->v4l2_dev.dev, "group probe failed: %d\n", ret);

return ret;
--
1.9.1


2015-02-18 17:59:03

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH] [media] soc-camera: Remove bogus devm_kfree() in soc_of_bind()

Hello.

On 02/18/2015 07:32 PM, Geert Uytterhoeven wrote:

> Unlike scan_async_group(), soc_of_bind() doesn't allocate its
> soc_camera_async_client structure using devm_kzalloc(), but has it
> embedded inside the soc_of_info structure. Hence on failure, it must
> not free it using devm_kfree(), as this will cause a warning, and may
> cause slab corruption:

[...]

> Fixes: 1ddc6a6caa94e1e1 ("[media] soc_camera: add support for dt binding soc_camera drivers")
> Cc: [email protected]
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> Triggered with shmobile-defconfig on r8a7791/koelsch.
> ---
> drivers/media/platform/soc_camera/soc_camera.c | 1 -
> 1 file changed, 1 deletion(-)

> diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
> index cee7b56f84049944..d8a072fe46035821 100644
> --- a/drivers/media/platform/soc_camera/soc_camera.c
> +++ b/drivers/media/platform/soc_camera/soc_camera.c
> @@ -1665,7 +1665,6 @@ eclkreg:
> eaddpdev:
> platform_device_put(sasc->pdev);
> eallocpdev:
> - devm_kfree(ici->v4l2_dev.dev, sasc);

Perhaps Ben meant 'info' ISO 'sasc'? This way it would make more sense.

> dev_err(ici->v4l2_dev.dev, "group probe failed: %d\n", ret);
>
> return ret;

WBR, Sergei

2015-02-18 21:17:15

by Guennadi Liakhovetski

[permalink] [raw]
Subject: Re: [PATCH] [media] soc-camera: Remove bogus devm_kfree() in soc_of_bind()

On Wed, 18 Feb 2015, Sergei Shtylyov wrote:

> Hello.
>
> On 02/18/2015 07:32 PM, Geert Uytterhoeven wrote:
>
> > Unlike scan_async_group(), soc_of_bind() doesn't allocate its
> > soc_camera_async_client structure using devm_kzalloc(), but has it
> > embedded inside the soc_of_info structure. Hence on failure, it must
> > not free it using devm_kfree(), as this will cause a warning, and may
> > cause slab corruption:
>
> [...]
>
> > Fixes: 1ddc6a6caa94e1e1 ("[media] soc_camera: add support for dt binding
> > soc_camera drivers")
> > Cc: [email protected]
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > ---
> > Triggered with shmobile-defconfig on r8a7791/koelsch.
> > ---
> > drivers/media/platform/soc_camera/soc_camera.c | 1 -
> > 1 file changed, 1 deletion(-)
>
> > diff --git a/drivers/media/platform/soc_camera/soc_camera.c
> > b/drivers/media/platform/soc_camera/soc_camera.c
> > index cee7b56f84049944..d8a072fe46035821 100644
> > --- a/drivers/media/platform/soc_camera/soc_camera.c
> > +++ b/drivers/media/platform/soc_camera/soc_camera.c
> > @@ -1665,7 +1665,6 @@ eclkreg:
> > eaddpdev:
> > platform_device_put(sasc->pdev);
> > eallocpdev:
> > - devm_kfree(ici->v4l2_dev.dev, sasc);
>
> Perhaps Ben meant 'info' ISO 'sasc'? This way it would make more sense.

Agree. Geert, could you double-check and respin?

Thanks
Guennadi

>
> > dev_err(ici->v4l2_dev.dev, "group probe failed: %d\n", ret);
> >
> > return ret;
>
> WBR, Sergei
>

2015-02-19 09:45:00

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] [media] soc-camera: Remove bogus devm_kfree() in soc_of_bind()

Hi Guennadi,

On Wed, Feb 18, 2015 at 10:16 PM, Guennadi Liakhovetski
<[email protected]> wrote:
> On Wed, 18 Feb 2015, Sergei Shtylyov wrote:
>> On 02/18/2015 07:32 PM, Geert Uytterhoeven wrote:
>> > Unlike scan_async_group(), soc_of_bind() doesn't allocate its
>> > soc_camera_async_client structure using devm_kzalloc(), but has it
>> > embedded inside the soc_of_info structure. Hence on failure, it must
>> > not free it using devm_kfree(), as this will cause a warning, and may
>> > cause slab corruption:
>>
>> [...]
>>
>> > Fixes: 1ddc6a6caa94e1e1 ("[media] soc_camera: add support for dt binding
>> > soc_camera drivers")
>> > Cc: [email protected]
>> > Signed-off-by: Geert Uytterhoeven <[email protected]>
>> > ---
>> > Triggered with shmobile-defconfig on r8a7791/koelsch.
>> > ---
>> > drivers/media/platform/soc_camera/soc_camera.c | 1 -
>> > 1 file changed, 1 deletion(-)
>>
>> > diff --git a/drivers/media/platform/soc_camera/soc_camera.c
>> > b/drivers/media/platform/soc_camera/soc_camera.c
>> > index cee7b56f84049944..d8a072fe46035821 100644
>> > --- a/drivers/media/platform/soc_camera/soc_camera.c
>> > +++ b/drivers/media/platform/soc_camera/soc_camera.c
>> > @@ -1665,7 +1665,6 @@ eclkreg:
>> > eaddpdev:
>> > platform_device_put(sasc->pdev);
>> > eallocpdev:
>> > - devm_kfree(ici->v4l2_dev.dev, sasc);
>>
>> Perhaps Ben meant 'info' ISO 'sasc'? This way it would make more sense.
>
> Agree. Geert, could you double-check and respin?

Sergei is right. Will update and resend.

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds