linux-kernel - Re: [PATCH] x86: fix IO APIC resource allocation error message

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <200903202100.30789.bzolnier@gmail.com>
Date:	Fri, 20 Mar 2009 21:00:30 +0100
From:	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Alan Bartlett <ajb.stxsl@...glemail.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86: fix IO APIC resource allocation error message

On Friday 20 March 2009, Ingo Molnar wrote:
> 
> * Bartlomiej Zolnierkiewicz <bzolnier@...il.com> wrote:
> 
> > From: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
> > Subject: [PATCH] x86: fix IO APIC resource allocation error message
> > 
> > Impact: fix incorrect error message
> > 
> > - IO APIC resource allocation error message contains one too many "be".
> > 
> > - Print the error message iff there are IO APICs in the system.
> > 
> > Cc: Alan Bartlett <ajb.stxsl@...glemail.com>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
> > ---
> > I've seen this error message for some time on my x86-32 laptop...
> > 
> >  arch/x86/kernel/io_apic.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > Index: b/arch/x86/kernel/io_apic.c
> > ===================================================================
> > --- a/arch/x86/kernel/io_apic.c
> > +++ b/arch/x86/kernel/io_apic.c
> > @@ -4150,9 +4150,9 @@ static int __init ioapic_insert_resource
> >  	int i;
> >  	struct resource *r = ioapic_resources;
> >  
> > -	if (!r) {
> > +	if (!r && nr_ioapics > 0) {
> >  		printk(KERN_ERR
> > -		       "IO APIC resources could be not be allocated.\n");
> > +		       "IO APIC resources couldn't be allocated.\n");
> >  		return -1;
> >  	}
> 
> looks good, but there's one weirdness:
> 
> so if nr_ioapics == 0 && !r we'll drop into this codepath:
> 
>         for (i = 0; i < nr_ioapics; i++) {
>                 insert_resource(&iomem_resource, r);
>                 r++;
>         }
> 
>         return 0;
> 
> we survive the loop by luck, and then return 0 - which we'll survive 

s/by luck/by design/ -- we want to return 0 and thus have the correct
return value of the initcall

> too but still it's a bit unexpected and hence fragile.

agreed on this one

> So i think we should rather add a standalone:
> 
> 	if (nr_ioapics > 0)
>   		printk(KERN_ERR
> 
> check to the printk only. That wont affect the remaining code flow.

here is the revised version

From: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
Subject: [PATCH] x86: fix IO APIC resource allocation error message (take 2)

Impact: fix incorrect error message

- IO APIC resource allocation error message contains one too many "be".

- Print the error message iff there are IO APICs in the system.

Cc: Alan Bartlett <ajb.stxsl@...glemail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
---
I've seen this error message for some time on my x86-32 laptop...

 arch/x86/kernel/io_apic.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Index: b/arch/x86/kernel/io_apic.c
===================================================================
--- a/arch/x86/kernel/io_apic.c
+++ b/arch/x86/kernel/io_apic.c
@@ -4151,9 +4151,12 @@ static int __init ioapic_insert_resource
 	struct resource *r = ioapic_resources;
 
 	if (!r) {
-		printk(KERN_ERR
-		       "IO APIC resources could be not be allocated.\n");
-		return -1;
+		if (nr_ioapics > 0) {
+			printk(KERN_ERR
+				"IO APIC resources couldn't be allocated.\n");
+			return -1;
+		}
+		return 0;
 	}
 
 	for (i = 0; i < nr_ioapics; i++) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/