[PATCH] arch/x86/kernel/apic/io_apic.c: Fix for crash when apic=debug is used

[PATCH] arch/x86/kernel/apic/io_apic.c: Fix for crash when apic=debug is used

[Daniel Kiper]

Hi,

Here is the patch fixing crash when apic=debug
is used and APIC is not properly initialized.
This issue appears during Xen Dom0 kernel boot
(git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git,
xen/stable-2.6.32.x head), however I think that
patch is rather generic and should be applied
to mainline kernel (it applies to Linux Kernel
Ver. 2.6.35 and Ver. 2.6.32.19 with small fuzz).

Daniel

Signed-off-by: Daniel Kiper <dkiper@net-space.pl>
---
 arch/x86/kernel/apic/io_apic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index e41ed24..2b18af1 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1728,6 +1728,8 @@ __apicdebuginit(void) print_IO_APIC(void)
 		struct irq_pin_list *entry;
 
 		cfg = desc->chip_data;
+		if (!cfg)
+			continue;
 		entry = cfg->irq_2_pin;
 		if (!entry)
 			continue;

[PATCH] arch/x86/kernel/apic/io_apic.c: Fix for crash when apic=debug is used

[Daniel Kiper]

Hi,

Here is the patch fixing crash when apic=debug
is used and APIC is not properly initialized.
This issue appears during Xen Dom0 kernel boot
(git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git,
xen/stable-2.6.32.x head), however I think that
patch is rather generic and should be applied
to mainline kernel (it applies to Linux Kernel
Ver. 2.6.35 and Ver. 2.6.32.19 with small fuzz).

Daniel

Signed-off-by: Daniel Kiper <dkiper@net-space.pl>
---
 arch/x86/kernel/apic/io_apic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index e41ed24..2b18af1 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1728,6 +1728,8 @@ __apicdebuginit(void) print_IO_APIC(void)
 		struct irq_pin_list *entry;
 
 		cfg = desc->chip_data;
+		if (!cfg)
+			continue;
 		entry = cfg->irq_2_pin;
 		if (!entry)
 			continue;

[tip:x86/urgent] x86, apic: Fix apic=debug boot crash

[tip-bot for Daniel Kiper]

Commit-ID:  05e407603e527f9d808dd3866d3a17c2ce4dfcc5
Gitweb:     http://git.kernel.org/tip/05e407603e527f9d808dd3866d3a17c2ce4dfcc5
Author:     Daniel Kiper <dkiper@net-space.pl>
AuthorDate: Fri, 20 Aug 2010 00:46:16 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 20 Aug 2010 10:18:28 +0200

x86, apic: Fix apic=debug boot crash

Fix a boot crash when apic=debug is used and the APIC is
not properly initialized.

This issue appears during Xen Dom0 kernel boot but the
fix is generic and the crash could occur on real hardware
as well.

Signed-off-by: Daniel Kiper <dkiper@net-space.pl>
Cc: xen-devel@lists.xensource.com
Cc: konrad.wilk@oracle.com
Cc: jeremy@goop.org
Cc: <stable@kernel.org> # .35.x, .34.x, .33.x, .32.x
LKML-Reference: <20100819224616.GB9967@router-fw-old.local.net-space.pl>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/apic/io_apic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 4dc0084..f1efeba 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1728,6 +1728,8 @@ __apicdebuginit(void) print_IO_APIC(void)
 		struct irq_pin_list *entry;
 
 		cfg = desc->chip_data;
+		if (!cfg)
+			continue;
 		entry = cfg->irq_2_pin;
 		if (!entry)
 			continue;

Re: [tip:x86/urgent] x86, apic: Fix apic=debug boot crash

[Yinghai Lu]

On Fri, Aug 20, 2010 at 5:41 AM, tip-bot for Daniel Kiper
<dkiper@net-space.pl> wrote:
> Commit-ID:  05e407603e527f9d808dd3866d3a17c2ce4dfcc5
> Gitweb:     http://git.kernel.org/tip/05e407603e527f9d808dd3866d3a17c2ce4dfcc5
> Author:     Daniel Kiper <dkiper@net-space.pl>
> AuthorDate: Fri, 20 Aug 2010 00:46:16 +0200
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Fri, 20 Aug 2010 10:18:28 +0200
>
> x86, apic: Fix apic=debug boot crash
>
> Fix a boot crash when apic=debug is used and the APIC is
> not properly initialized.
>
> This issue appears during Xen Dom0 kernel boot but the
> fix is generic and the crash could occur on real hardware
> as well.

Do you have any report on real hardware?

that could not happen on real hardware.

Yinghai

>
> Signed-off-by: Daniel Kiper <dkiper@net-space.pl>
> Cc: xen-devel@lists.xensource.com
> Cc: konrad.wilk@oracle.com
> Cc: jeremy@goop.org
> Cc: <stable@kernel.org> # .35.x, .34.x, .33.x, .32.x
> LKML-Reference: <20100819224616.GB9967@router-fw-old.local.net-space.pl>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  arch/x86/kernel/apic/io_apic.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 4dc0084..f1efeba 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -1728,6 +1728,8 @@ __apicdebuginit(void) print_IO_APIC(void)
>                struct irq_pin_list *entry;
>
>                cfg = desc->chip_data;
> +               if (!cfg)
> +                       continue;
>                entry = cfg->irq_2_pin;
>                if (!entry)
>                        continue;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@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: [tip:x86/urgent] x86, apic: Fix apic=debug boot crash

[Daniel Kiper]

Hi,

On Fri, Aug 20, 2010 at 12:24:20PM -0700, Yinghai Lu wrote:
> On Fri, Aug 20, 2010 at 5:41 AM, tip-bot for Daniel Kiper
> <dkiper@net-space.pl> wrote:
> > Commit-ID: ?05e407603e527f9d808dd3866d3a17c2ce4dfcc5
> > Gitweb: ? ? http://git.kernel.org/tip/05e407603e527f9d808dd3866d3a17c2ce4dfcc5
> > Author: ? ? Daniel Kiper <dkiper@net-space.pl>
> > AuthorDate: Fri, 20 Aug 2010 00:46:16 +0200
> > Committer: ?Ingo Molnar <mingo@elte.hu>
> > CommitDate: Fri, 20 Aug 2010 10:18:28 +0200
> >
> > x86, apic: Fix apic=debug boot crash
> >
> > Fix a boot crash when apic=debug is used and the APIC is
> > not properly initialized.
> >
> > This issue appears during Xen Dom0 kernel boot but the
> > fix is generic and the crash could occur on real hardware
> > as well.
>
> Do you have any report on real hardware?
> that could not happen on real hardware.

Till now no, however I think it is good idea
to apply this patch now. It is not worth to wait
for another null pointer dereference.

Daniel

Re: [tip:x86/urgent] x86, apic: Fix apic=debug boot crash

[H. Peter Anvin]

It's already applied.

"Daniel Kiper" <dkiper@net-space.pl> wrote:

>Hi,
>
>On Fri, Aug 20, 2010 at 12:24:20PM -0700, Yinghai Lu wrote:
>> On Fri, Aug 20, 2010 at 5:41 AM, tip-bot for Daniel Kiper
>> <dkiper@net-space.pl> wrote:
>> > Commit-ID: ?05e407603e527f9d808dd3866d3a17c2ce4dfcc5
>> > Gitweb: ? ? http://git.kernel.org/tip/05e407603e527f9d808dd3866d3a17c2ce4dfcc5
>> > Author: ? ? Daniel Kiper <dkiper@net-space.pl>
>> > AuthorDate: Fri, 20 Aug 2010 00:46:16 +0200
>> > Committer: ?Ingo Molnar <mingo@elte.hu>
>> > CommitDate: Fri, 20 Aug 2010 10:18:28 +0200
>> >
>> > x86, apic: Fix apic=debug boot crash
>> >
>> > Fix a boot crash when apic=debug is used and the APIC is
>> > not properly initialized.
>> >
>> > This issue appears during Xen Dom0 kernel boot but the
>> > fix is generic and the crash could occur on real hardware
>> > as well.
>>
>> Do you have any report on real hardware?
>> that could not happen on real hardware.
>
>Till now no, however I think it is good idea
>to apply this patch now. It is not worth to wait
>for another null pointer dereference.
>
>Daniel

-- 
Sent from my mobile phone.  Please pardon any lack of formatting.

Re: [tip:x86/urgent] x86, apic: Fix apic=debug boot crash

[Yinghai Lu]

On 08/23/2010 07:54 AM, H. Peter Anvin wrote:
> It's already applied.
> 
> "Daniel Kiper" <dkiper@net-space.pl> wrote:
> 
>> Hi,
>>
>> On Fri, Aug 20, 2010 at 12:24:20PM -0700, Yinghai Lu wrote:
>>> On Fri, Aug 20, 2010 at 5:41 AM, tip-bot for Daniel Kiper
>>> <dkiper@net-space.pl> wrote:
>>>> Commit-ID: ?05e407603e527f9d808dd3866d3a17c2ce4dfcc5
>>>> Gitweb: ? ? http://git.kernel.org/tip/05e407603e527f9d808dd3866d3a17c2ce4dfcc5
>>>> Author: ? ? Daniel Kiper <dkiper@net-space.pl>
>>>> AuthorDate: Fri, 20 Aug 2010 00:46:16 +0200
>>>> Committer: ?Ingo Molnar <mingo@elte.hu>
>>>> CommitDate: Fri, 20 Aug 2010 10:18:28 +0200
>>>>
>>>> x86, apic: Fix apic=debug boot crash
>>>>
>>>> Fix a boot crash when apic=debug is used and the APIC is
>>>> not properly initialized.
>>>>
>>>> This issue appears during Xen Dom0 kernel boot but the
>>>> fix is generic and the crash could occur on real hardware
>>>> as well.
>>>
>>> Do you have any report on real hardware?
>>> that could not happen on real hardware.
>>
>> Till now no, however I think it is good idea
>> to apply this patch now. It is not worth to wait
>> for another null pointer dereference.

no, we should add BUG_ON() etc debug info there to see why that null cfg could happen.

because according to code, we should have null there.

Yinghai

Re: [tip:x86/urgent] x86, apic: Fix apic=debug boot crash

[Daniel Kiper]

Hello,

On 08/23/2010 07:54 AM, H. Peter Anvin wrote:
> It's already applied.

Thx.

On Mon, Aug 23, 2010 at 10:59:19AM -0700, Yinghai Lu wrote:
[...]
> >>>> x86, apic: Fix apic=debug boot crash
> >>>>
> >>>> Fix a boot crash when apic=debug is used and the APIC is
> >>>> not properly initialized.
> >>>>
> >>>> This issue appears during Xen Dom0 kernel boot but the
> >>>> fix is generic and the crash could occur on real hardware
> >>>> as well.
> >>>
> >>> Do you have any report on real hardware?
> >>> that could not happen on real hardware.
> >>
> >> Till now no, however I think it is good idea
> >> to apply this patch now. It is not worth to wait
> >> for another null pointer dereference.
>
> no, we should add BUG_ON() etc debug info there to see why that null cfg could happen.
> because according to code, we should have null there.

I think that BUG_ON() is too strong here because
it is "debug" function and it should work also
with let's say "invalid" data (in Xen case it is
normal because APIC state is managed directly
by hypervisor).

Additionally, with this patch it is easy to
differentiate between cfg != NULL and
cfg == NULL. Please look below:

cfg != NULL:
IRQ to pin mappings:
IRQ0 -> 0:2
IRQ1 -> 0:1
IRQ3 -> 0:3
IRQ4 -> 0:4
IRQ5 -> 0:5
IRQ6 -> 0:6
IRQ7 -> 0:7
IRQ8 -> 0:8
IRQ9 -> 0:9
IRQ10 -> 0:10
IRQ11 -> 0:11
IRQ12 -> 0:12
IRQ13 -> 0:13
IRQ14 -> 0:14
IRQ15 -> 0:15
.................................... done.

cfg == NULL:
IRQ to pin mappings:
.................................... done.

If I missed something or if you have any
questions please drop me a line.

Daniel

Re: [tip:x86/urgent] x86, apic: Fix apic=debug boot crash

[Yinghai Lu]

On 08/24/2010 02:39 PM, Daniel Kiper wrote:
> Hello,
> 
> On 08/23/2010 07:54 AM, H. Peter Anvin wrote:
>> It's already applied.
> 
> Thx.
> 
> On Mon, Aug 23, 2010 at 10:59:19AM -0700, Yinghai Lu wrote:
> [...]
>>>>>> x86, apic: Fix apic=debug boot crash
>>>>>>
>>>>>> Fix a boot crash when apic=debug is used and the APIC is
>>>>>> not properly initialized.
>>>>>>
>>>>>> This issue appears during Xen Dom0 kernel boot but the
>>>>>> fix is generic and the crash could occur on real hardware
>>>>>> as well.
>>>>>
>>>>> Do you have any report on real hardware?
>>>>> that could not happen on real hardware.
>>>>
>>>> Till now no, however I think it is good idea
>>>> to apply this patch now. It is not worth to wait
>>>> for another null pointer dereference.
>>
>> no, we should add BUG_ON() etc debug info there to see why that null cfg could happen.
>> because according to code, we should have null there.
> 
> I think that BUG_ON() is too strong here because
> it is "debug" function and it should work also
> with let's say "invalid" data (in Xen case it is
> normal because APIC state is managed directly
> by hypervisor).
> 
> Additionally, with this patch it is easy to
> differentiate between cfg != NULL and
> cfg == NULL. Please look below:
> 
> cfg != NULL:
> IRQ to pin mappings:
> IRQ0 -> 0:2
> IRQ1 -> 0:1
> IRQ3 -> 0:3
> IRQ4 -> 0:4
> IRQ5 -> 0:5
> IRQ6 -> 0:6
> IRQ7 -> 0:7
> IRQ8 -> 0:8
> IRQ9 -> 0:9
> IRQ10 -> 0:10
> IRQ11 -> 0:11
> IRQ12 -> 0:12
> IRQ13 -> 0:13
> IRQ14 -> 0:14
> IRQ15 -> 0:15
> .................................... done.
> 
> cfg == NULL:
> IRQ to pin mappings:
> .................................... done.
> 
> If I missed something or if you have any
> questions please drop me a line.

I mean you should figure out why xen ops could have null cfg.

Yinghai

Re: [tip:x86/urgent] x86, apic: Fix apic=debug boot crash

[Daniel Kiper]

Hello,

On Tue, Aug 24, 2010 at 02:47:44PM -0700, Yinghai Lu wrote:
[...]
> I mean you should figure out why xen ops could have null cfg.

OK, I will check that next week.

Daniel