linux-kernel - Re: [PATCH v2] x86/apic: Fix modify

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 23 Aug 2016 13:31:07 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Wanpeng Li <kernellwp@...il.com>
cc:     linux-kernel@...r.kernel.org, x86@...nel.org,
        Wanpeng Li <wanpeng.li@...mail.com>,
        Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        "H. Peter Anvin" <hpa@...or.com>, Joerg Roedel <joro@...tes.org>
Subject: Re: [PATCH v2] x86/apic: Fix modify_irte NULL pointer

On Tue, 23 Aug 2016, Wanpeng Li wrote:

> From: Wanpeng Li <wanpeng.li@...mail.com>
> 
> native_smp_prepare_cpus
>   -> default_setup_apic_routing
>     -> enable_IR_x2apic
>       -> irq_remapping_prepare
>         -> intel_prepare_irq_remapping
>           -> intel_setup_irq_remapping		  
> 
> IR table is setup even if noapic boot parameter is added.
> 
> As a result:
> 
>     BUG: unable to handle kernel NULL pointer dereference at           (null)
>     IP: [<ffffffff8d5a5e58>] modify_irte+0x58/0x140
>     PGD 209638067 PUD 2105f4067 PMD 0 
>     Oops: 0000 [#1] SMP
>     RIP: 0010:[<ffffffff8d5a5e58>]  [<ffffffff8d5a5e58>] modify_irte+0x58/0x140
>     Call Trace:
>      intel_ir_set_affinity+0xa3/0xb0
>      msi_domain_set_affinity+0x21/0x70
>      ? __irq_set_affinity+0x34/0x70
>      irq_do_set_affinity+0x1d/0x70
>      irq_set_affinity_locked+0xc2/0x100
>      __irq_set_affinity+0x47/0x70
>      write_irq_affinity.isra.7+0xcc/0xf0
>      irq_affinity_proc_write+0x19/0x20
>      proc_reg_write+0x3d/0x70
>      ? rcu_sync_lockdep_assert+0x2f/0x60
>      __vfs_write+0x28/0x120
>      ? percpu_down_read+0x5c/0xa0
>      ? __sb_start_write+0xca/0xe0
>      ? __sb_start_write+0xca/0xe0
>      vfs_write+0xb5/0x1b0
>      SyS_write+0x49/0xa0
>      do_syscall_64+0x81/0x220
>      entry_SYSCALL64_slow_path+0x25/0x25
>     RIP  [<ffffffff8d5a5e58>] modify_irte+0x58/0x140
>      RSP <ffff8e9ad01b7c78>
>     CR2: 0000000000000000
>     
> irqbalance is running at the end of booting and changes the irq affinity, 
> then irte is flushed. We should not have MSI and such if apic is disabled. 
> This patch fix it by return -ENODEV if apic is disabled in order to avoid 
> to setup ir table for ioapic.

I don't see any ENODEV here in the patch. This changelog sucks. The problem is
neither irqbalance nor MSI. The problem is simply that we try to setup irq
remapping even if "noapic" is on the kernel command line and therefor ioapic
support is disabled.

> @@ -154,6 +154,9 @@ void __init default_setup_apic_routing(void)
>  {
>  	int version = apic_version[boot_cpu_physical_apicid];
>  
> +	if (skip_ioapic_setup)
> +		return;

So you skip the whole APIC setup as well. APIC != IOAPIC. This check belongs
into enable_IR_x2apic().

Thanks,

	tglx