From: Helen Koike <helen.koike@collabora.com>
To: Shuah Khan <skhan@linuxfoundation.org>,
mchehab@kernel.org, andrealmeid@collabora.com,
dafna.hirschfeld@collabora.com, hverkuil-cisco@xs4all.nl
Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 2/5] media: vimc: Fix gpf in rmmod path when stream is active
Date: Sun, 15 Sep 2019 16:25:36 -0300 [thread overview]
Message-ID: <05f955ad-525a-cabf-637f-d68a1ac43e33@collabora.com> (raw)
In-Reply-To: <2421a0ea4bec8c5084121ecdbc9213c331239f3d.1567822793.git.skhan@linuxfoundation.org>
On 9/6/19 11:42 PM, Shuah Khan wrote:
> If vimc module is removed while streaming is in progress, sensor subdev
> unregister runs into general protection fault when it tries to unregister
> media entities. This is a common subdev problem related to releasing
> pads from v4l2_device_unregister_subdev() before calling unregister.
> Unregister references pads during unregistering subdev.
>
> The sd release handler is the right place for releasing all sd resources
> including pads. The release handlers currently release all resources
> except the pads.
>
> Fix v4l2_device_unregister_subdev() not release pads and release pads
> from the sd_int_op release handlers.
>
> kernel: [ 4136.715839] general protection fault: 0000 [#1] SMP PTI
> kernel: [ 4136.715847] CPU: 2 PID: 1972 Comm: bash Not tainted 5.3.0-rc2+ #4
> kernel: [ 4136.715850] Hardware name: Dell Inc. OptiPlex 790/0HY9JP, BIOS A18 09/24/2013
> kernel: [ 4136.715858] RIP: 0010:media_gobj_destroy.part.16+0x1f/0x60
> kernel: [ 4136.715863] Code: ff 66 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 90 55 48 89 fe 48 89 e5 53 48 89 fb 48 c7 c7 00 7f cf b0 e8 24 fa ff ff 48 8b 03 <48> 83 80 a0 00 00 00 01 48 8b 43 18 48 8b 53 10 48 89 42 08 48 89
> kernel: [ 4136.715866] RSP: 0018:ffff9b2248fe3cb0 EFLAGS: 00010246
> kernel: [ 4136.715870] RAX: bcf2bfbfa0d63c2f RBX: ffff88c3eb37e9c0 RCX: 00000000802a0018
> kernel: [ 4136.715873] RDX: ffff88c3e4f6a078 RSI: ffff88c3eb37e9c0 RDI: ffffffffb0cf7f00
> kernel: [ 4136.715876] RBP: ffff9b2248fe3cb8 R08: 0000000001000002 R09: ffffffffb0492b00
> kernel: [ 4136.715879] R10: ffff9b2248fe3c28 R11: 0000000000000001 R12: 0000000000000038
> kernel: [ 4136.715881] R13: ffffffffc09a1628 R14: ffff88c3e4f6a028 R15: fffffffffffffff2
> kernel: [ 4136.715885] FS: 00007f8389647740(0000) GS:ffff88c465500000(0000) knlGS:0000000000000000
> kernel: [ 4136.715888] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> kernel: [ 4136.715891] CR2: 000055d008f80fd8 CR3: 00000001996ec005 CR4: 00000000000606e0
> kernel: [ 4136.715894] Call Trace:
> kernel: [ 4136.715903] media_gobj_destroy+0x14/0x20
> kernel: [ 4136.715908] __media_device_unregister_entity+0xb3/0xe0
> kernel: [ 4136.715915] media_device_unregister_entity+0x30/0x40
> kernel: [ 4136.715920] v4l2_device_unregister_subdev+0xa8/0xe0
> kernel: [ 4136.715928] vimc_ent_sd_unregister+0x1e/0x30 [vimc]
> kernel: [ 4136.715933] vimc_sen_rm+0x16/0x20 [vimc]
> kernel: [ 4136.715938] vimc_remove+0x3e/0xa0 [vimc]
> kernel: [ 4136.715945] platform_drv_remove+0x25/0x50
> kernel: [ 4136.715951] device_release_driver_internal+0xe0/0x1b0
> kernel: [ 4136.715956] device_driver_detach+0x14/0x20
> kernel: [ 4136.715960] unbind_store+0xd1/0x130
> kernel: [ 4136.715965] drv_attr_store+0x27/0x40
> kernel: [ 4136.715971] sysfs_kf_write+0x48/0x60
> kernel: [ 4136.715976] kernfs_fop_write+0x128/0x1b0
> kernel: [ 4136.715982] __vfs_write+0x1b/0x40
> kernel: [ 4136.715987] vfs_write+0xc3/0x1d0
> kernel: [ 4136.715993] ksys_write+0xaa/0xe0
> kernel: [ 4136.715999] __x64_sys_write+0x1a/0x20
> kernel: [ 4136.716005] do_syscall_64+0x5a/0x130
> kernel: [ 4136.716010] entry_SYSCALL_64_after_hwframe+0x4
> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Acked-by: Helen Koike <helen.koike@collabora.com>
> ---
> drivers/media/platform/vimc/vimc-common.c | 3 +--
> drivers/media/platform/vimc/vimc-debayer.c | 1 +
> drivers/media/platform/vimc/vimc-scaler.c | 1 +
> drivers/media/platform/vimc/vimc-sensor.c | 1 +
> 4 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/vimc/vimc-common.c b/drivers/media/platform/vimc/vimc-common.c
> index 7e1ae0b12f1e..a3120f4f7a90 100644
> --- a/drivers/media/platform/vimc/vimc-common.c
> +++ b/drivers/media/platform/vimc/vimc-common.c
> @@ -375,7 +375,7 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
> {
> int ret;
>
> - /* Allocate the pads */
> + /* Allocate the pads. Should be released from the sd_int_op release */
> ved->pads = vimc_pads_init(num_pads, pads_flag);
> if (IS_ERR(ved->pads))
> return PTR_ERR(ved->pads);
> @@ -424,7 +424,6 @@ EXPORT_SYMBOL_GPL(vimc_ent_sd_register);
> void vimc_ent_sd_unregister(struct vimc_ent_device *ved, struct v4l2_subdev *sd)
> {
> media_entity_cleanup(ved->ent);
> - vimc_pads_cleanup(ved->pads);
> v4l2_device_unregister_subdev(sd);
> }
> EXPORT_SYMBOL_GPL(vimc_ent_sd_unregister);
> diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
> index 00850f2501ad..b38b55f51a24 100644
> --- a/drivers/media/platform/vimc/vimc-debayer.c
> +++ b/drivers/media/platform/vimc/vimc-debayer.c
> @@ -482,6 +482,7 @@ static void vimc_deb_release(struct v4l2_subdev *sd)
> struct vimc_deb_device *vdeb =
> container_of(sd, struct vimc_deb_device, sd);
>
> + vimc_pads_cleanup(vdeb->ved.pads);
> kfree(vdeb);
> }
>
> diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
> index 28a947950a0b..05db5070e268 100644
> --- a/drivers/media/platform/vimc/vimc-scaler.c
> +++ b/drivers/media/platform/vimc/vimc-scaler.c
> @@ -338,6 +338,7 @@ static void vimc_sca_release(struct v4l2_subdev *sd)
> struct vimc_sca_device *vsca =
> container_of(sd, struct vimc_sca_device, sd);
>
> + vimc_pads_cleanup(vsca->ved.pads);
> kfree(vsca);
> }
>
> diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
> index 1f15637ca8bb..46dc6a535abe 100644
> --- a/drivers/media/platform/vimc/vimc-sensor.c
> +++ b/drivers/media/platform/vimc/vimc-sensor.c
> @@ -291,6 +291,7 @@ static void vimc_sen_release(struct v4l2_subdev *sd)
>
> v4l2_ctrl_handler_free(&vsen->hdl);
> tpg_free(&vsen->tpg);
> + vimc_pads_cleanup(vsen->ved.pads);
> kfree(vsen);
> }
>
>
next prev parent reply other threads:[~2019-09-15 19:25 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-07 2:42 [PATCH v4 0/5] Collapse vimc into single monolithic driver Shuah Khan
2019-09-07 2:42 ` [PATCH v4 1/5] media: vimc: Collapse component structure into a " Shuah Khan
2019-09-15 19:25 ` Helen Koike
2019-09-16 0:40 ` Shuah Khan
2019-09-07 2:42 ` [PATCH v4 2/5] media: vimc: Fix gpf in rmmod path when stream is active Shuah Khan
2019-09-15 19:25 ` Helen Koike [this message]
2019-09-07 2:42 ` [PATCH v4 3/5] vimc: move duplicated IS_SRC and IS_SINK to common header Shuah Khan
2019-09-15 19:25 ` Helen Koike
2019-09-15 23:52 ` Shuah Khan
2019-09-16 10:42 ` Helen Koike
2019-09-17 2:42 ` Shuah Khan
2019-09-07 2:42 ` [PATCH v4 4/5] doc: media: vimc: Update module parameter usage information Shuah Khan
2019-09-15 19:25 ` Helen Koike
2019-09-07 2:42 ` [PATCH v4 5/5] MAINTAINERS: Add reviewer to vimc driver Shuah Khan
2019-09-15 19:26 ` Helen Koike
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=05f955ad-525a-cabf-637f-d68a1ac43e33@collabora.com \
--to=helen.koike@collabora.com \
--cc=andrealmeid@collabora.com \
--cc=dafna.hirschfeld@collabora.com \
--cc=hverkuil-cisco@xs4all.nl \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=skhan@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).