Hi, Tomasz,
Thanks for your patch.
________________________
BRs,
Bingbu Cao
> -----Original Message-----
> From: Tomasz Figa <tfiga@xxxxxxxxxxxx>
> Sent: Sunday, April 12, 2020 1:37 AM
> To: linux-media@xxxxxxxxxxxxxxx
> Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>; Mauro Carvalho Chehab
> <mchehab@xxxxxxxxxx>; Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Qiu,
> Tian Shu <tian.shu.qiu@xxxxxxxxx>; Cao, Bingbu <bingbu.cao@xxxxxxxxx>;
> devel@xxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Laurent Pinchart
> <laurent.pinchart@xxxxxxxxxxxxxxxx>; jacopo mondi <jacopo@xxxxxxxxxx>;
> Tomasz Figa <tfiga@xxxxxxxxxxxx>
> Subject: [PATCH] media: staging: ipu3: Fix stale list entries on parameter
> queue failure
>
> When queuing parameters fails, current code bails out without deleting the
> corresponding vb2 buffer from the driver buffer list, but the buffer is
> returned to vb2. This leads to stale list entries and a crash when the
> driver stops streaming:
>
> [ 224.935561] ipu3-imgu 0000:00:05.0: set parameters failed.
> [ 224.998932] ipu3-imgu 0000:00:05.0: set parameters failed.
> [ 225.064430] ipu3-imgu 0000:00:05.0: set parameters failed.
> [ 225.128534] ipu3-imgu 0000:00:05.0: set parameters failed.
> [ 225.194945] ipu3-imgu 0000:00:05.0: set parameters failed.
> [ 225.360363] ------------[ cut here ]------------ [ 225.360372] WARNING:
> CPU: 0 PID: 6704 at
> drivers/media/common/videobuf2/videobuf2-core.c:927
> vb2_buffer_done+0x20f/0x21a [videobuf2_common] [ 225.360374] Modules linked
> in: snd_seq_dummy snd_seq snd_seq_device veth bridge stp llc tun nf_nat_tftp
> nf_conntrack_tftp nf_nat_ftp nf_conntrack_ftp esp6 ah6 ip6t_REJECT
> ip6t_ipv6header cmac rfcomm uinput
> ipu3_imgu(C) ipu3_cio2 iova videobuf2_v4l2 videobuf2_common videobuf2_dma_sg
> videobuf2_memops ov13858 ov5670 v4l2_fwnode dw9714 acpi_als xt_MASQUERADE
> fuse iio_trig_sysfs cros_ec_sensors_ring cros_ec_light_prox cros_ec_sensors
> cros_ec_sensors_core industrialio_triggered_buffer kfifo_buf industrialio
> cros_ec_sensorsupport cdc_ether btusb btrtl btintel btbcm usbnet bluetooth
> ecdh_generic ecc hid_google_hammer iwlmvm iwl7000_mac80211
> r8152 mii lzo_rle lzo_compress iwlwifi zram cfg80211 joydev [ 225.360400]
> CPU: 0 PID: 6704 Comm: CameraDeviceOps Tainted: G
> C 5.4.30 #5
> [ 225.360402] Hardware name: HP Soraka/Soraka, BIOS
> Google_Soraka.10431.106.0 12/03/2019
> [ 225.360405] RIP: 0010:vb2_buffer_done+0x20f/0x21a [videobuf2_common]
> [ 225.360408] Code: 5e 41 5f 5d e9 e0 16 5a d4 41 8b 55 08 48 c7 c7 8f 8b
> 5c c0 48 c7 c6 36 9a 5c c0 44 89 f9 31 c0 e8 a5 1c 5b d4 e9 53 fe ff ff <0f>
> 0b eb a3 e8 12 d7 43 d4 eb 97 0f 1f 44 00 00 55 48 89 e5 41 56 [ 225.360410]
> RSP: 0018:ffff9468ab32fba8 EFLAGS: 00010297 [ 225.360412] RAX:
> ffff8aa7a51577a8 RBX: dead000000000122 RCX:
> ffff8aa7a51577a8
> [ 225.360414] RDX: 0000000000000000 RSI: 0000000000000006 RDI:
> ffff8aa7a5157400
> [ 225.360416] RBP: ffff9468ab32fbd8 R08: ffff8aa64e47e600 R09:
> 0000000000000000
> [ 225.360418] R10: 0000000000000000 R11: ffffffffc06036e6 R12:
> dead000000000100
> [ 225.360420] R13: ffff8aa7820f1940 R14: ffff8aa7a51577a8 R15:
> 0000000000000006
> [ 225.360422] FS: 00007c1146ffd700(0000) GS:ffff8aa7baa00000(0000)
> knlGS:0000000000000000
> [ 225.360424] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 225.360426] CR2: 00007aea3473a000 CR3: 00000000537d6004 CR4:
> 00000000003606f0
> [ 225.360427] Call Trace:
> [ 225.360434] imgu_return_all_buffers+0x6f/0x8e [ipu3_imgu] [ 225.360438]
> imgu_vb2_stop_streaming+0xd6/0xf0 [ipu3_imgu] [ 225.360441]
> __vb2_queue_cancel+0x33/0x22d [videobuf2_common] [ 225.360443]
> vb2_core_streamoff+0x16/0x78 [videobuf2_common] [ 225.360448]
> __video_do_ioctl+0x33d/0x42a [ 225.360452] video_usercopy+0x34a/0x615
> [ 225.360455] ? video_ioctl2+0x16/0x16 [ 225.360458]
> v4l2_ioctl+0x46/0x53 [ 225.360462] do_vfs_ioctl+0x50a/0x787 [ 225.360465]
> ksys_ioctl+0x58/0x83 [ 225.360468] __x64_sys_ioctl+0x1a/0x1e [ 225.360470]
> do_syscall_64+0x54/0x68 [ 225.360474]
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [ 225.360476] RIP: 0033:0x7c118030f497
> [ 225.360479] Code: 8a 66 90 48 8b 05 d1 d9 2b 00 64 c7 00 26 00 00 00
> 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 0f
> 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d a1 d9 2b 00 f7 d8 64 89 01 48
> [ 225.360480] RSP: 002b:00007c1146ffa5a8 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000010
> [ 225.360483] RAX: ffffffffffffffda RBX: 00007c1140010018 RCX:
> 00007c118030f497
> [ 225.360484] RDX: 00007c114001019c RSI: 0000000040045613 RDI:
> 000000000000004c
> [ 225.360486] RBP: 00007c1146ffa700 R08: 00007c1140010048 R09:
> 0000000000000000
> [ 225.360488] R10: 0000000000000000 R11: 0000000000000246 R12:
> 00007c11400101b0
> [ 225.360489] R13: 00007c1140010200 R14: 00007c1140010048 R15:
> 0000000000000001
> [ 225.360492] ---[ end trace 73625ecfbd1c930e ]--- [ 225.360498] general
> protection fault: 0000 [#1] PREEMPT SMP PTI [ 225.360501] CPU: 0 PID: 6704
> Comm: CameraDeviceOps Tainted: G
> WC 5.4.30 #5
> [ 225.360502] Hardware name: HP Soraka/Soraka, BIOS
> Google_Soraka.10431.106.0 12/03/2019
> [ 225.360505] RIP: 0010:imgu_return_all_buffers+0x52/0x8e [ipu3_imgu]
> [ 225.360507] Code: d4 49 8b 85 70 0a 00 00 49 81 c5 70 0a 00 00 49 39
> c5 74 3b 49 bc 00 01 00 00 00 00 ad de 49 8d 5c 24 22 4c 8b 30 48 8b 48
> 08 <49> 89 4e 08 4c 89 31 4c 89 20 48 89 58 08 48 8d b8 58 fc ff ff 44
> [ 225.360509] RSP: 0018:ffff9468ab32fbe8 EFLAGS: 00010293 [ 225.360511]
> RAX: ffff8aa7a51577a8 RBX: dead000000000122 RCX:
> dead000000000122
> [ 225.360512] RDX: 0000000000000000 RSI: 0000000000000006 RDI:
> ffff8aa7a5157400
> [ 225.360514] RBP: ffff9468ab32fc18 R08: ffff8aa64e47e600 R09:
> 0000000000000000
> [ 225.360515] R10: 0000000000000000 R11: ffffffffc06036e6 R12:
> dead000000000100
> [ 225.360517] R13: ffff8aa7820f1940 R14: dead000000000100 R15:
> 0000000000000006
> [ 225.360519] FS: 00007c1146ffd700(0000) GS:ffff8aa7baa00000(0000)
> knlGS:0000000000000000
> [ 225.360521] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 225.360523] CR2: 00007aea3473a000 CR3: 00000000537d6004 CR4:
> 00000000003606f0
> [ 225.360525] Call Trace:
> [ 225.360528] imgu_vb2_stop_streaming+0xd6/0xf0 [ipu3_imgu] [ 225.360531]
> __vb2_queue_cancel+0x33/0x22d [videobuf2_common] [ 225.360534]
> vb2_core_streamoff+0x16/0x78 [videobuf2_common] [ 225.360537]
> __video_do_ioctl+0x33d/0x42a [ 225.360540] video_usercopy+0x34a/0x615
> [ 225.360542] ? video_ioctl2+0x16/0x16 [ 225.360546]
> v4l2_ioctl+0x46/0x53 [ 225.360548] do_vfs_ioctl+0x50a/0x787 [ 225.360551]
> ksys_ioctl+0x58/0x83 [ 225.360554] __x64_sys_ioctl+0x1a/0x1e [ 225.360556]
> do_syscall_64+0x54/0x68 [ 225.360559]
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [ 225.360561] RIP: 0033:0x7c118030f497
> [ 225.360563] Code: 8a 66 90 48 8b 05 d1 d9 2b 00 64 c7 00 26 00 00 00
> 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 0f
> 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d a1 d9 2b 00 f7 d8 64 89 01 48
> [ 225.360565] RSP: 002b:00007c1146ffa5a8 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000010
> [ 225.360567] RAX: ffffffffffffffda RBX: 00007c1140010018 RCX:
> 00007c118030f497
> [ 225.360569] RDX: 00007c114001019c RSI: 0000000040045613 RDI:
> 000000000000004c
> [ 225.360570] RBP: 00007c1146ffa700 R08: 00007c1140010048 R09:
> 0000000000000000
> [ 225.360572] R10: 0000000000000000 R11: 0000000000000246 R12:
> 00007c11400101b0
> [ 225.360574] R13: 00007c1140010200 R14: 00007c1140010048 R15:
> 0000000000000001
> [ 225.360576] Modules linked in: snd_seq_dummy snd_seq snd_seq_device veth
> bridge stp llc tun nf_nat_tftp nf_conntrack_tftp nf_nat_ftp nf_conntrack_ftp
> esp6 ah6 ip6t_REJECT ip6t_ipv6header cmac rfcomm uinput
> ipu3_imgu(C) ipu3_cio2 iova videobuf2_v4l2 videobuf2_common videobuf2_dma_sg
> videobuf2_memops ov13858 ov567
>
> Fix this by moving the list_del() call just below the list_first_entry()
> call when the buffer no longer needs to be in the list.
>
> Fixes: 8ecc7c9da013 ("media: staging/intel-ipu3: parameter buffer
> refactoring")
> Signed-off-by: Tomasz Figa <tfiga@xxxxxxxxxxxx>
> ---
> drivers/staging/media/ipu3/ipu3.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/ipu3/ipu3.c
> b/drivers/staging/media/ipu3/ipu3.c
> index 4d53aad31483..7a1d1881483b 100644
> --- a/drivers/staging/media/ipu3/ipu3.c
> +++ b/drivers/staging/media/ipu3/ipu3.c
> @@ -261,6 +261,7 @@ int imgu_queue_buffers(struct imgu_device *imgu, bool
> initial, unsigned int pipe
>
> ivb = list_first_entry(&imgu_pipe->nodes[node].buffers,
> struct imgu_vb2_buffer, list);
> + list_del(&ivb->list);
> vb = &ivb->vbb.vb2_buf;
> r = imgu_css_set_parameters(&imgu->css, pipe,
> vb2_plane_vaddr(vb, 0));
> @@ -274,7 +275,6 @@ int imgu_queue_buffers(struct imgu_device *imgu, bool
> initial, unsigned int pipe
> vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
> dev_dbg(&imgu->pci_dev->dev,
> "queue user parameters %d to css.", vb->index);
> - list_del(&ivb->list);
> } else if (imgu_pipe->queue_enabled[node]) {
> struct imgu_css_buffer *buf =
> imgu_queue_getbuf(imgu, node, pipe);
> --
Reviewed-by: Bingbu Cao <bingbu.cao@xxxxxxxxx>
> 2.26.0.110.g2183baf09c-goog
