Hi, Laurent
Thanks for your reply!
We allocated the context when init_videobuf2() which will be called in soc_camera_open(), so if we get exit with exception in soc_camera_set_fmt()
Actually we will double call vb2_dma_contig_cleanup_ctx().
ret = soc_camera_set_fmt(icd, &f);
if (ret < 0)
goto esfmt;
if (ici->ops->init_videobuf) {
ici->ops->init_videobuf(&icd->vb_vidq, icd);
} else {
ret = ici->ops->init_videobuf2(&icd->vb2_vidq, icd);
if (ret < 0)
goto einitvb;
}
Actually, in current code, we can found some drivers allocated the context in probe(), and some drivers also do that in soc_camera_open().
Of course, we can update our driver and move it to probe(), it will stand aside the issue, that's also OK for us.
But we still think it's not safe that leave the point be a non-NULL after we have kfree it. Do you think so?
Thanks
Albert Wang
86-21-61092656
-----Original Message-----
From: Laurent Pinchart [mailto:laurent.pinchart@xxxxxxxxxxxxxxxx]
Sent: Monday, 11 June, 2012 16:00
To: Albert Wang
Cc: pawel@xxxxxxxxxx; g.liakhovetski@xxxxxx; linux-media@xxxxxxxxxxxxxxx
Subject: Re: [PATCH] media: videobuf2: fix kernel panic due to missing assign NULL to alloc_ctx
Hi Albert,
On Friday 08 June 2012 19:55:11 Albert Wang wrote:
> In function vb2_dma_contig_cleanup_ctx(), we only kfree the alloc_ctx
> If we didn't assign NULL to this point after kfree it,
> we may encounter the following kernel panic:
>
> kernel BUG at kernel/cred.c:98!
> Unable to handle kernel NULL pointer dereference at virtual address
> 00000000 pgd = c0004000
> [00000000] *pgd=00000000
> Internal error: Oops: 817 [#1] PREEMPT SMP Modules linked in:
> runcase_sysfs galcore mv_wtm_drv mv_wtm_prim
> CPU: 0 Not tainted (3.0.8+ #213)
> PC is at __bug+0x18/0x24
> LR is at __bug+0x14/0x24
> pc : [<c0054670>] lr : [<c005466c>] psr: 60000113
> sp : c0681ec0 ip : f683e000 fp : 00000000
> r10: e8ab4b58 r9 : 00000fff r8 : 00000002
> r7 : e8665698 r6 : c10079ec r5 : e8b13d80 r4 : e8b13d98
> r3 : 00000000 r2 : c0681eb4 r1 : c05c9ccc r0 : 00000035
> Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
> Control: 10c53c7d Table: 29c3406a DAC: 00000015
>
> the root cause is we may encounter some i2c or HW issue with sensor
> which result in driver exit with exception during soc_camera_set_fmt()
> from soc_camera_open():
>
> ret = soc_camera_set_fmt(icd, &f);
> if (ret < 0)
> goto esfmt;
>
> it will call ici->ops->remove() in following code:
>
> esfmt:
> pm_runtime_disable(&icd->vdev->dev);
> eresume:
> ici->ops->remove(icd);
>
> ici->ops->remove() will call vb2_dma_contig_cleanup_ctx() for cleanup
> but we didn't do ici->ops->init_videobuf2() yet at that time
> it will result in kfree a non-NULL point twice
I'm not sure to follow you. How is init_videobuf2() related ? The context is allocated once only at probe time from what I can see. Your problem is more likely caused by a double call to vb2_dma_contig_cleanup_ctx(), which looks like a driver bug to me, not a videobuf2 bug.
> Change-Id: I1c66dd08438ae90abe555c52edcdbca0d39d829d
> Signed-off-by: Albert Wang <twang13@xxxxxxxxxxx>
> ---
> drivers/media/video/videobuf2-dma-contig.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/media/video/videobuf2-dma-contig.c
> b/drivers/media/video/videobuf2-dma-contig.c index 4b71326..9881171
> 100755
> --- a/drivers/media/video/videobuf2-dma-contig.c
> +++ b/drivers/media/video/videobuf2-dma-contig.c
> @@ -178,6 +178,7 @@ EXPORT_SYMBOL_GPL(vb2_dma_contig_init_ctx);
> void vb2_dma_contig_cleanup_ctx(void *alloc_ctx) {
> kfree(alloc_ctx);
> + alloc_ctx = NULL;
> }
> EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
