[B/D/E][SRU][PATCH 0/1] Fix for vimc null pointer dereference

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[B/D/E][SRU][PATCH 0/1] Fix for vimc null pointer dereference

Po-Hsu Lin (Sam)
BugLink: https://bugs.launchpad.net/bugs/1840028

== SRU Justification ==
When trying to insert a vimc module on a system has other devices being
registered in the component framework, if the device is not necessarily
a platform_device, nor have a platform_data it will trigger a NULL
pointer deference issue.

Issue found on a bare metal node with config vimc enabled.

ubuntu@amaura:~$ sudo modprobe vimc
Killed

dmesg output:
[ 2855.340272] media: Linux media interface: v0.10
[ 2855.344927] Linux video capture interface: v2.00
[ 2855.346146] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
[ 2855.346172] IP: strcmp+0xe/0x30
[ 2855.346181] PGD 0 P4D 0
[ 2855.346189] Oops: 0000 [#1] SMP PTI
[ 2855.346198] Modules linked in: vimc(+) videodev media ppdev intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel binfmt_misc kvm irqbypass intel_cstate intel_rapl_perf ipmi_si joydev ipmi_devintf ipmi_msghandler intel_pch_thermal input_leds parport_pc lpc_ich shpchp parport mac_hid sch_fq_codel ib_iser rdma_cm iw_cm ib_cm ib_core iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi ip_tables x_tables autofs4 btrfs zstd_compress raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq libcrc32c raid1 raid0 multipath linear hid_generic usbhid hid crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc i915 mgag200 ttm drm_kms_helper aesni_intel syscopyarea aes_x86_64 sysfillrect crypto_simd igb sysimgblt glue_helper fb_sys_fops cryptd dca drm i2c_algo_bit
[ 2855.346366] ahci ptp libahci pps_core video
[ 2855.346379] CPU: 4 PID: 1505 Comm: modprobe Not tainted 4.15.0-58-generic #64
[ 2855.346395] Hardware name: Intel Corporation S1200RP/S1200RP, BIOS S1200RP.86B.03.02.0003.070120151022 07/01/2015
[ 2855.346418] RIP: 0010:strcmp+0xe/0x30
[ 2855.346428] RSP: 0018:ffffb63501f93a00 EFLAGS: 00010202
[ 2855.346440] RAX: ffffffffc0c860f0 RBX: 0000000000000000 RCX: 0000000000000000
[ 2855.346456] RDX: ffffa097d85ec440 RSI: ffffffffc0c8723f RDI: 0000000000000001
[ 2855.346473] RBP: ffffb63501f93a00 R08: ffffa097e09270a0 R09: ffffa097d265ca80
[ 2855.346489] R10: ffffe84b51559600 R11: 0000000000000200 R12: ffffa097dcdbf718
[ 2855.346505] R13: ffffa097d265ca80 R14: ffffa097d2f2b380 R15: 0000000000000000
[ 2855.346521] FS: 00007fd7f4e4b540(0000) GS:ffffa097e0900000(0000) knlGS:0000000000000000
[ 2855.346539] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 2855.346553] CR2: 0000000000000000 CR3: 00000004580fc001 CR4: 00000000003606e0
[ 2855.346569] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 2855.346585] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 2855.346601] Call Trace:
[ 2855.346611] vimc_comp_compare+0x15/0x20 [vimc]
[ 2855.346624] try_to_bring_up_master+0xa3/0x260
[ 2855.346635] ? vimc_remove+0x90/0x90 [vimc]
[ 2855.346646] component_master_add_with_match+0x8b/0xd0
[ 2855.346659] vimc_probe+0x325/0x3c9 [vimc]
[ 2855.346672] ? acpi_dev_pm_attach+0x25/0xd0
[ 2855.346683] platform_drv_probe+0x3e/0xa0
[ 2855.346693] driver_probe_device+0x30c/0x490
[ 2855.346704] __driver_attach+0xa7/0xf0
[ 2855.346714] ? driver_probe_device+0x490/0x490
[ 2855.346725] bus_for_each_dev+0x70/0xc0
[ 2855.346735] driver_attach+0x1e/0x20
[ 2855.346744] bus_add_driver+0x1c7/0x270
[ 2855.346754] ? 0xffffffffc0c8b000
[ 2855.346763] driver_register+0x60/0xe0
[ 2855.346772] ? 0xffffffffc0c8b000
[ 2855.346781] __platform_driver_register+0x36/0x40
[ 2855.346793] vimc_init+0x46/0x1000 [vimc]
[ 2855.347306] do_one_initcall+0x52/0x19f
[ 2855.347810] ? __vunmap+0x8e/0xc0
[ 2855.348322] ? _cond_resched+0x19/0x40
[ 2855.348811] ? kmem_cache_alloc_trace+0x14e/0x1b0
[ 2855.349290] ? do_init_module+0x27/0x209
[ 2855.349768] do_init_module+0x5f/0x209
[ 2855.350246] load_module+0x193b/0x1f30
[ 2855.350710] ? ima_post_read_file+0x96/0xa0
[ 2855.351159] SYSC_finit_module+0xfc/0x120
[ 2855.351592] ? SYSC_finit_module+0xfc/0x120
[ 2855.352010] SyS_finit_module+0xe/0x10
[ 2855.352412] do_syscall_64+0x73/0x130
[ 2855.352797] entry_SYSCALL_64_after_hwframe+0x3d/0xa2
[ 2855.353169] RIP: 0033:0x7fd7f4959839
[ 2855.353538] RSP: 002b:00007ffd7e3fd5c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[ 2855.353915] RAX: ffffffffffffffda RBX: 0000563c3b02eea0 RCX: 00007fd7f4959839
[ 2855.354286] RDX: 0000000000000000 RSI: 0000563c39de5d2e RDI: 0000000000000005
[ 2855.354647] RBP: 0000563c39de5d2e R08: 0000000000000000 R09: 0000563c3b02eea0
[ 2855.355009] R10: 0000000000000005 R11: 0000000000000246 R12: 0000000000000000
[ 2855.355369] R13: 0000563c3b02ef20 R14: 0000000000040000 R15: 0000563c3b02eea0
[ 2855.355728] Code: 01 c8 c3 c6 44 07 ff 00 eb 91 31 c0 eb c9 48 c7 c0 f9 ff ff ff c3 0f 1f 80 00 00 00 00 55 48 89 e5 eb 04 84 c0 74 18 48 83 c7 01 <0f> b6 47 ff 48 83 c6 01 3a 46 ff 74 eb 19 c0 83 c8 01 5d c3 31
[ 2855.356503] RIP: strcmp+0xe/0x30 RSP: ffffb63501f93a00
[ 2855.356885] CR2: 0000000000000000
[ 2855.357259] ---[ end trace bfba48c80f803d2d ]---

== Fix ==
* ee1c71a8 (media: vimc: fix component match compare)

This patch can be cherry-picked in to B/D/E.
VIMC support was requested to enabled on these kernels (lp:1831482).

== Test ==
Test kernels could be found here:
https://people.canonical.com/~phlin/kernel/lp-1840028-null-ptr-vimc/

Tested with node "amaura", patch works as expected, the vimc module can
be inserted / removed without any issue.

== Regression Potential ==
Low, this patch is specific for vimc and we have positive test result
with it.

Helen Koike (1):
  media: vimc: fix component match compare

 drivers/media/platform/vimc/vimc-core.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[B/D/E][SRU][PATCH 1/1] media: vimc: fix component match compare

Po-Hsu Lin (Sam)
From: Helen Koike <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1840028

If the system has other devices being registered in the component
framework, the compare function will be called with a device that
doesn't belong to vimc.
This device is not necessarily a platform_device, nor have a
platform_data (which causes a NULL pointer dereference error) and if it
does have a pdata, it is not necessarily type of struct vimc_platform_data.
So casting to any of these types is wrong.

Instead of expecting a given pdev with a given pdata, just expect for
the device it self. vimc-core is the one who creates them, we know in
advance exactly which object to expect in the match.

Fixes: 4a29b7090749 ("[media] vimc: Subdevices as modules")

Signed-off-by: Helen Koike <[hidden email]>
Reviewed-by: Boris Brezillon <[hidden email]>
Tested-by: Boris Brezillon <[hidden email]>
Signed-off-by: Hans Verkuil <[hidden email]>
Signed-off-by: Mauro Carvalho Chehab <[hidden email]>
(cherry picked from commit ee1c71a8e1456ab53fe667281d855849edf26a4d)
Signed-off-by: Po-Hsu Lin <[hidden email]>
---
 drivers/media/platform/vimc/vimc-core.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-core.c b/drivers/media/platform/vimc/vimc-core.c
index 64eb424..b48813e 100644
--- a/drivers/media/platform/vimc/vimc-core.c
+++ b/drivers/media/platform/vimc/vimc-core.c
@@ -243,10 +243,7 @@ static void vimc_comp_unbind(struct device *master)
 
 static int vimc_comp_compare(struct device *comp, void *data)
 {
- const struct platform_device *pdev = to_platform_device(comp);
- const char *name = data;
-
- return !strcmp(pdev->dev.platform_data, name);
+ return comp == data;
 }
 
 static struct component_match *vimc_add_subdevs(struct vimc_device *vimc)
@@ -276,7 +273,7 @@ static struct component_match *vimc_add_subdevs(struct vimc_device *vimc)
  }
 
  component_match_add(&vimc->pdev.dev, &match, vimc_comp_compare,
-    (void *)vimc->pipe_cfg->ents[i].name);
+    &vimc->subdevs[i]->dev);
  }
 
  return match;
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [B/D/E][SRU][PATCH 1/1] media: vimc: fix component match compare

Colin Ian King-2
On 14/08/2019 09:48, Po-Hsu Lin wrote:

> From: Helen Koike <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1840028
>
> If the system has other devices being registered in the component
> framework, the compare function will be called with a device that
> doesn't belong to vimc.
> This device is not necessarily a platform_device, nor have a
> platform_data (which causes a NULL pointer dereference error) and if it
> does have a pdata, it is not necessarily type of struct vimc_platform_data.
> So casting to any of these types is wrong.
>
> Instead of expecting a given pdev with a given pdata, just expect for
> the device it self. vimc-core is the one who creates them, we know in
> advance exactly which object to expect in the match.
>
> Fixes: 4a29b7090749 ("[media] vimc: Subdevices as modules")
>
> Signed-off-by: Helen Koike <[hidden email]>
> Reviewed-by: Boris Brezillon <[hidden email]>
> Tested-by: Boris Brezillon <[hidden email]>
> Signed-off-by: Hans Verkuil <[hidden email]>
> Signed-off-by: Mauro Carvalho Chehab <[hidden email]>
> (cherry picked from commit ee1c71a8e1456ab53fe667281d855849edf26a4d)
> Signed-off-by: Po-Hsu Lin <[hidden email]>
> ---
>  drivers/media/platform/vimc/vimc-core.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/vimc/vimc-core.c b/drivers/media/platform/vimc/vimc-core.c
> index 64eb424..b48813e 100644
> --- a/drivers/media/platform/vimc/vimc-core.c
> +++ b/drivers/media/platform/vimc/vimc-core.c
> @@ -243,10 +243,7 @@ static void vimc_comp_unbind(struct device *master)
>  
>  static int vimc_comp_compare(struct device *comp, void *data)
>  {
> - const struct platform_device *pdev = to_platform_device(comp);
> - const char *name = data;
> -
> - return !strcmp(pdev->dev.platform_data, name);
> + return comp == data;
>  }
>  
>  static struct component_match *vimc_add_subdevs(struct vimc_device *vimc)
> @@ -276,7 +273,7 @@ static struct component_match *vimc_add_subdevs(struct vimc_device *vimc)
>   }
>  
>   component_match_add(&vimc->pdev.dev, &match, vimc_comp_compare,
> -    (void *)vimc->pipe_cfg->ents[i].name);
> +    &vimc->subdevs[i]->dev);
>   }
>  
>   return match;
>
Clean cherry pick

Acked-by: Colin Ian King <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK / APPLIED[E]: [B/D/E][SRU][PATCH 0/1] Fix for vimc null pointer dereference

Seth Forshee
In reply to this post by Po-Hsu Lin (Sam)
On Wed, Aug 14, 2019 at 04:48:02PM +0800, Po-Hsu Lin wrote:

> BugLink: https://bugs.launchpad.net/bugs/1840028
>
> == SRU Justification ==
> When trying to insert a vimc module on a system has other devices being
> registered in the component framework, if the device is not necessarily
> a platform_device, nor have a platform_data it will trigger a NULL
> pointer deference issue.
>
> Issue found on a bare metal node with config vimc enabled.
>
> ubuntu@amaura:~$ sudo modprobe vimc
> Killed
>
> dmesg output:
> [ 2855.340272] media: Linux media interface: v0.10
> [ 2855.344927] Linux video capture interface: v2.00
> [ 2855.346146] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
> [ 2855.346172] IP: strcmp+0xe/0x30
> [ 2855.346181] PGD 0 P4D 0
> [ 2855.346189] Oops: 0000 [#1] SMP PTI
> [ 2855.346198] Modules linked in: vimc(+) videodev media ppdev intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel binfmt_misc kvm irqbypass intel_cstate intel_rapl_perf ipmi_si joydev ipmi_devintf ipmi_msghandler intel_pch_thermal input_leds parport_pc lpc_ich shpchp parport mac_hid sch_fq_codel ib_iser rdma_cm iw_cm ib_cm ib_core iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi ip_tables x_tables autofs4 btrfs zstd_compress raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq libcrc32c raid1 raid0 multipath linear hid_generic usbhid hid crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc i915 mgag200 ttm drm_kms_helper aesni_intel syscopyarea aes_x86_64 sysfillrect crypto_simd igb sysimgblt glue_helper fb_sys_fops cryptd dca drm i2c_algo_bit
> [ 2855.346366] ahci ptp libahci pps_core video
> [ 2855.346379] CPU: 4 PID: 1505 Comm: modprobe Not tainted 4.15.0-58-generic #64
> [ 2855.346395] Hardware name: Intel Corporation S1200RP/S1200RP, BIOS S1200RP.86B.03.02.0003.070120151022 07/01/2015
> [ 2855.346418] RIP: 0010:strcmp+0xe/0x30
> [ 2855.346428] RSP: 0018:ffffb63501f93a00 EFLAGS: 00010202
> [ 2855.346440] RAX: ffffffffc0c860f0 RBX: 0000000000000000 RCX: 0000000000000000
> [ 2855.346456] RDX: ffffa097d85ec440 RSI: ffffffffc0c8723f RDI: 0000000000000001
> [ 2855.346473] RBP: ffffb63501f93a00 R08: ffffa097e09270a0 R09: ffffa097d265ca80
> [ 2855.346489] R10: ffffe84b51559600 R11: 0000000000000200 R12: ffffa097dcdbf718
> [ 2855.346505] R13: ffffa097d265ca80 R14: ffffa097d2f2b380 R15: 0000000000000000
> [ 2855.346521] FS: 00007fd7f4e4b540(0000) GS:ffffa097e0900000(0000) knlGS:0000000000000000
> [ 2855.346539] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 2855.346553] CR2: 0000000000000000 CR3: 00000004580fc001 CR4: 00000000003606e0
> [ 2855.346569] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 2855.346585] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 2855.346601] Call Trace:
> [ 2855.346611] vimc_comp_compare+0x15/0x20 [vimc]
> [ 2855.346624] try_to_bring_up_master+0xa3/0x260
> [ 2855.346635] ? vimc_remove+0x90/0x90 [vimc]
> [ 2855.346646] component_master_add_with_match+0x8b/0xd0
> [ 2855.346659] vimc_probe+0x325/0x3c9 [vimc]
> [ 2855.346672] ? acpi_dev_pm_attach+0x25/0xd0
> [ 2855.346683] platform_drv_probe+0x3e/0xa0
> [ 2855.346693] driver_probe_device+0x30c/0x490
> [ 2855.346704] __driver_attach+0xa7/0xf0
> [ 2855.346714] ? driver_probe_device+0x490/0x490
> [ 2855.346725] bus_for_each_dev+0x70/0xc0
> [ 2855.346735] driver_attach+0x1e/0x20
> [ 2855.346744] bus_add_driver+0x1c7/0x270
> [ 2855.346754] ? 0xffffffffc0c8b000
> [ 2855.346763] driver_register+0x60/0xe0
> [ 2855.346772] ? 0xffffffffc0c8b000
> [ 2855.346781] __platform_driver_register+0x36/0x40
> [ 2855.346793] vimc_init+0x46/0x1000 [vimc]
> [ 2855.347306] do_one_initcall+0x52/0x19f
> [ 2855.347810] ? __vunmap+0x8e/0xc0
> [ 2855.348322] ? _cond_resched+0x19/0x40
> [ 2855.348811] ? kmem_cache_alloc_trace+0x14e/0x1b0
> [ 2855.349290] ? do_init_module+0x27/0x209
> [ 2855.349768] do_init_module+0x5f/0x209
> [ 2855.350246] load_module+0x193b/0x1f30
> [ 2855.350710] ? ima_post_read_file+0x96/0xa0
> [ 2855.351159] SYSC_finit_module+0xfc/0x120
> [ 2855.351592] ? SYSC_finit_module+0xfc/0x120
> [ 2855.352010] SyS_finit_module+0xe/0x10
> [ 2855.352412] do_syscall_64+0x73/0x130
> [ 2855.352797] entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> [ 2855.353169] RIP: 0033:0x7fd7f4959839
> [ 2855.353538] RSP: 002b:00007ffd7e3fd5c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
> [ 2855.353915] RAX: ffffffffffffffda RBX: 0000563c3b02eea0 RCX: 00007fd7f4959839
> [ 2855.354286] RDX: 0000000000000000 RSI: 0000563c39de5d2e RDI: 0000000000000005
> [ 2855.354647] RBP: 0000563c39de5d2e R08: 0000000000000000 R09: 0000563c3b02eea0
> [ 2855.355009] R10: 0000000000000005 R11: 0000000000000246 R12: 0000000000000000
> [ 2855.355369] R13: 0000563c3b02ef20 R14: 0000000000040000 R15: 0000563c3b02eea0
> [ 2855.355728] Code: 01 c8 c3 c6 44 07 ff 00 eb 91 31 c0 eb c9 48 c7 c0 f9 ff ff ff c3 0f 1f 80 00 00 00 00 55 48 89 e5 eb 04 84 c0 74 18 48 83 c7 01 <0f> b6 47 ff 48 83 c6 01 3a 46 ff 74 eb 19 c0 83 c8 01 5d c3 31
> [ 2855.356503] RIP: strcmp+0xe/0x30 RSP: ffffb63501f93a00
> [ 2855.356885] CR2: 0000000000000000
> [ 2855.357259] ---[ end trace bfba48c80f803d2d ]---
>
> == Fix ==
> * ee1c71a8 (media: vimc: fix component match compare)
>
> This patch can be cherry-picked in to B/D/E.
> VIMC support was requested to enabled on these kernels (lp:1831482).
>
> == Test ==
> Test kernels could be found here:
> https://people.canonical.com/~phlin/kernel/lp-1840028-null-ptr-vimc/
>
> Tested with node "amaura", patch works as expected, the vimc module can
> be inserted / removed without any issue.
>
> == Regression Potential ==
> Low, this patch is specific for vimc and we have positive test result
> with it.

Acked-by: Seth Forshee <[hidden email]>

Applied to eoan/master-next, thanks!

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team