Re: [Patch Part1 V2 12/17] iommu/vt-d: fix invalid memory access when freeing DMAR irq
Reviewed-by: Yijing Wang
On 2013/11/29 16:50, Jiang Liu wrote:
> In function free_dmar_iommu(), it sets IRQ handler data to NULL
> before calling free_irq(), which will cause invalid memory access
> because free_irq() will access IRQ handler data when calling
> function dmar_msi_mask(). So only set IRQ handler data to NULL
> after calling free_irq().
>
> Sample stack dump:
> [ 13.094010] BUG: unable to handle kernel NULL pointer dereference at
> 0048
> [ 13.103215] IP: [] __lock_acquire+0x4d/0x12a0
> [ 13.110104] PGD 0
> [ 13.112614] Oops: [#1] SMP
> [ 13.116585] Modules linked in:
> [ 13.120260] CPU: 60 PID: 1 Comm: swapper/0 Tainted: GW
> 3.13.0-rc1-gerry+ #9
> [ 13.129367] Hardware name: Intel Corporation LH Pass
> ../SVRBD-ROW_T, BIOS SE5C600.86B.99.99.x059.091020121352 09/10/2012
> [ 13.142555] task: 88042dd38010 ti: 88042dd32000 task.ti:
> 88042dd32000
> [ 13.151179] RIP: 0010:[] []
> __lock_acquire+0x4d/0x12a0
> [ 13.160867] RSP: :88042dd33b78 EFLAGS: 00010046
> [ 13.166969] RAX: 0046 RBX: 0002 RCX:
>
> [ 13.175122] RDX: RSI: RDI:
> 0048
> [ 13.183274] RBP: 88042dd33bd8 R08: 0002 R09:
> 0001
> [ 13.191417] R10: R11: 0001 R12:
> 88042dd38010
> [ 13.199571] R13: R14: 0048 R15:
>
> [ 13.207725] FS: () GS:88103f20()
> knlGS:
> [ 13.217014] CS: 0010 DS: ES: CR0: 80050033
> [ 13.223596] CR2: 0048 CR3: 01a0b000 CR4:
> 000407e0
> [ 13.231747] Stack:
> [ 13.234160] 0004 0046 88042dd33b98
> 810a567d
> [ 13.243059] 88042dd33c08 810bb14c 828995a0
> 0046
> [ 13.251969] 0002
>
> [ 13.260862] Call Trace:
> [ 13.263775] [] ? trace_hardirqs_off+0xd/0x10
> [ 13.270571] [] ? vprintk_emit+0x23c/0x570
> [ 13.277058] [] lock_acquire+0x93/0x120
> [ 13.283269] [] ? dmar_msi_mask+0x47/0x70
> [ 13.289677] [] _raw_spin_lock_irqsave+0x49/0x90
> [ 13.296748] [] ? dmar_msi_mask+0x47/0x70
> [ 13.303153] [] dmar_msi_mask+0x47/0x70
> [ 13.309354] [] irq_shutdown+0x53/0x60
> [ 13.315467] [] __free_irq+0x26d/0x280
> [ 13.321580] [] free_irq+0xf0/0x180
> [ 13.327395] [] free_dmar_iommu+0x271/0x2b0
> [ 13.333996] [] ? trace_hardirqs_on+0xd/0x10
> [ 13.340696] [] free_iommu+0x17/0x50
> [ 13.346597] [] init_dmars+0x691/0x77a
> [ 13.352711] [] intel_iommu_init+0x351/0x438
> [ 13.359400] [] ? iommu_setup+0x27d/0x27d
> [ 13.365806] [] pci_iommu_init+0x28/0x52
> [ 13.372114] [] do_one_initcall+0x122/0x180
> [ 13.378707] [] ? parse_args+0x1e8/0x320
> [ 13.385016] [] kernel_init_freeable+0x1e1/0x26c
> [ 13.392100] [] ? do_early_param+0x88/0x88
> [ 13.398596] [] ? rest_init+0xd0/0xd0
> [ 13.404614] [] kernel_init+0xe/0x130
> [ 13.410626] [] ret_from_fork+0x7c/0xb0
> [ 13.416829] [] ? rest_init+0xd0/0xd0
> [ 13.422842] Code: ec 99 00 85 c0 8b 05 53 05 a5 00 41 0f 45 d8 85 c0 0f 84
> ff 00 00 00 8b 05 99 f9 7e 01 49 89 fe 41 89 f7 85 c0 0f 84 03 01 00 00 <49>
> 8b 06 be 01 00 00 00 48 3d c0 0e 01 82 0f 44 de 41 83 ff 01
> [ 13.450191] RIP [] __lock_acquire+0x4d/0x12a0
> [ 13.458598] RSP
> [ 13.462671] CR2: 0048
> [ 13.466551] ---[ end trace c5bd26a37c81d760 ]---
>
> Signed-off-by: Jiang Liu
> ---
> drivers/iommu/intel-iommu.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 0ec49da..426095e 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -1289,9 +1289,9 @@ void free_dmar_iommu(struct intel_iommu *iommu)
> iommu_disable_translation(iommu);
>
> if (iommu->irq) {
> - irq_set_handler_data(iommu->irq, NULL);
> /* This will mask the irq */
> free_irq(iommu->irq, iommu);
> + irq_set_handler_data(iommu->irq, NULL);
> destroy_irq(iommu->irq);
> }
>
>
--
Thanks!
Yijing
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch Part1 V2 12/17] iommu/vt-d: fix invalid memory access when freeing DMAR irq
Reviewed-by: Yijing Wang wangyij...@huawei.com
On 2013/11/29 16:50, Jiang Liu wrote:
In function free_dmar_iommu(), it sets IRQ handler data to NULL
before calling free_irq(), which will cause invalid memory access
because free_irq() will access IRQ handler data when calling
function dmar_msi_mask(). So only set IRQ handler data to NULL
after calling free_irq().
Sample stack dump:
[ 13.094010] BUG: unable to handle kernel NULL pointer dereference at
0048
[ 13.103215] IP: [810a97cd] __lock_acquire+0x4d/0x12a0
[ 13.110104] PGD 0
[ 13.112614] Oops: [#1] SMP
[ 13.116585] Modules linked in:
[ 13.120260] CPU: 60 PID: 1 Comm: swapper/0 Tainted: GW
3.13.0-rc1-gerry+ #9
[ 13.129367] Hardware name: Intel Corporation LH Pass
../SVRBD-ROW_T, BIOS SE5C600.86B.99.99.x059.091020121352 09/10/2012
[ 13.142555] task: 88042dd38010 ti: 88042dd32000 task.ti:
88042dd32000
[ 13.151179] RIP: 0010:[810a97cd] [810a97cd]
__lock_acquire+0x4d/0x12a0
[ 13.160867] RSP: :88042dd33b78 EFLAGS: 00010046
[ 13.166969] RAX: 0046 RBX: 0002 RCX:
[ 13.175122] RDX: RSI: RDI:
0048
[ 13.183274] RBP: 88042dd33bd8 R08: 0002 R09:
0001
[ 13.191417] R10: R11: 0001 R12:
88042dd38010
[ 13.199571] R13: R14: 0048 R15:
[ 13.207725] FS: () GS:88103f20()
knlGS:
[ 13.217014] CS: 0010 DS: ES: CR0: 80050033
[ 13.223596] CR2: 0048 CR3: 01a0b000 CR4:
000407e0
[ 13.231747] Stack:
[ 13.234160] 0004 0046 88042dd33b98
810a567d
[ 13.243059] 88042dd33c08 810bb14c 828995a0
0046
[ 13.251969] 0002
[ 13.260862] Call Trace:
[ 13.263775] [810a567d] ? trace_hardirqs_off+0xd/0x10
[ 13.270571] [810bb14c] ? vprintk_emit+0x23c/0x570
[ 13.277058] [810ab1e3] lock_acquire+0x93/0x120
[ 13.283269] [814623f7] ? dmar_msi_mask+0x47/0x70
[ 13.289677] [8156b449] _raw_spin_lock_irqsave+0x49/0x90
[ 13.296748] [814623f7] ? dmar_msi_mask+0x47/0x70
[ 13.303153] [814623f7] dmar_msi_mask+0x47/0x70
[ 13.309354] [810c0d93] irq_shutdown+0x53/0x60
[ 13.315467] [810bdd9d] __free_irq+0x26d/0x280
[ 13.321580] [810be920] free_irq+0xf0/0x180
[ 13.327395] [81466591] free_dmar_iommu+0x271/0x2b0
[ 13.333996] [810a947d] ? trace_hardirqs_on+0xd/0x10
[ 13.340696] [81461a17] free_iommu+0x17/0x50
[ 13.346597] [81dc75a5] init_dmars+0x691/0x77a
[ 13.352711] [81dc7afd] intel_iommu_init+0x351/0x438
[ 13.359400] [81d8a711] ? iommu_setup+0x27d/0x27d
[ 13.365806] [81d8a739] pci_iommu_init+0x28/0x52
[ 13.372114] [81000342] do_one_initcall+0x122/0x180
[ 13.378707] [81077738] ? parse_args+0x1e8/0x320
[ 13.385016] [81d850e8] kernel_init_freeable+0x1e1/0x26c
[ 13.392100] [81d84833] ? do_early_param+0x88/0x88
[ 13.398596] [8154f8b0] ? rest_init+0xd0/0xd0
[ 13.404614] [8154f8be] kernel_init+0xe/0x130
[ 13.410626] [81574d6c] ret_from_fork+0x7c/0xb0
[ 13.416829] [8154f8b0] ? rest_init+0xd0/0xd0
[ 13.422842] Code: ec 99 00 85 c0 8b 05 53 05 a5 00 41 0f 45 d8 85 c0 0f 84
ff 00 00 00 8b 05 99 f9 7e 01 49 89 fe 41 89 f7 85 c0 0f 84 03 01 00 00 49
8b 06 be 01 00 00 00 48 3d c0 0e 01 82 0f 44 de 41 83 ff 01
[ 13.450191] RIP [810a97cd] __lock_acquire+0x4d/0x12a0
[ 13.458598] RSP 88042dd33b78
[ 13.462671] CR2: 0048
[ 13.466551] ---[ end trace c5bd26a37c81d760 ]---
Signed-off-by: Jiang Liu jiang@linux.intel.com
---
drivers/iommu/intel-iommu.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 0ec49da..426095e 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1289,9 +1289,9 @@ void free_dmar_iommu(struct intel_iommu *iommu)
iommu_disable_translation(iommu);
if (iommu-irq) {
- irq_set_handler_data(iommu-irq, NULL);
/* This will mask the irq */
free_irq(iommu-irq, iommu);
+ irq_set_handler_data(iommu-irq, NULL);
destroy_irq(iommu-irq);
}
--
Thanks!
Yijing
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ
