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: <20090511082258.GC5636@elte.hu>
Date:	Mon, 11 May 2009 10:22:58 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Yinghai Lu <yinghai@...nel.org>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Jesse Barnes <jbarnes@...tuousgeek.org>,
	Len Brown <lenb@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-pci@...r.kernel.org,
	ACPI Devel Maling List <linux-acpi@...r.kernel.org>
Subject: Re: [PATCH 3/7] x86: fix alloc_mptable


* Yinghai Lu <yinghai@...nel.org> wrote:

> fix the condition checking.
> 
> [ Impact: make alloc_mptable working ]
> 
> Signed-off-by: Yinghai Lu <yinghai@...nel.org>
> 
> ---
>  arch/x86/kernel/mpparse.c |   15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> Index: linux-2.6/arch/x86/kernel/mpparse.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/mpparse.c
> +++ linux-2.6/arch/x86/kernel/mpparse.c
> @@ -873,21 +873,24 @@ inline void __init check_irq_src(struct
>  static int check_slot(unsigned long mpc_new_phys, unsigned long mpc_new_length,
>  		      int count)
>  {
> +	int ret = 0;
> +
>  	if (!mpc_new_phys) {
> -		pr_info("No spare slots, try to append...take your risk, "
> +		pr_warning("No spare slots, try to append...take your risk, "
>  			"new mpc_length %x\n", count);
>  	} else {
> -		if (count <= mpc_new_length)
> +		if (count <= mpc_new_length) {
>  			pr_info("No spare slots, try to append..., "
>  				"new mpc_length %x\n", count);
> -		else {
> +			ret = 1;
> +		} else {
>  			pr_err("mpc_new_length %lx is too small\n",
>  				mpc_new_length);
> -			return -1;
> +			ret = -1;
>  		}
>  	}
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int  __init replace_intsrc_all(struct mpc_table *mpc,
> @@ -946,7 +949,7 @@ static int  __init replace_intsrc_all(st
>  		} else {
>  			struct mpc_intsrc *m = (struct mpc_intsrc *)mpt;
>  			count += sizeof(struct mpc_intsrc);
> -			if (!check_slot(mpc_new_phys, mpc_new_length, count))
> +			if (check_slot(mpc_new_phys, mpc_new_length, count) < 0)
>  				goto out;
>  			assign_to_mpc_intsrc(&mp_irqs[i], m);
>  			mpc->length = count;

hm, i modified this to the code attached below instead. Things like:

> -		pr_info("No spare slots, try to append...take your risk, "
> +		pr_warning("No spare slots, try to append...take your risk, "
>  			"new mpc_length %x\n", count);

are not acceptable at all. If there's _anything_ wrong with code 
like this, if we run out of a static pool of slots, we dont try to 
hack our way out of it... Instead we inform the user and bail out 
ASAP!

A predictable, well working way out of a resource shortage is _way_ 
more important than trying to clinge to some functionality and 
hoping that it will be all fine ...

So ... please describe under which conditions we can run out of 
slots, and what the options are in that situation.

	Ingo

------------------------------>
Subject: x86: fix alloc_mptable()
From: Yinghai Lu <yinghai@...nel.org>
Date: Wed, 06 May 2009 10:07:07 -0700

Fix the conditions when we stop updating the mptable due to
running out of slots.

[ Impact: fix memory corruption / non-working update_mptable boot parameter ]

Signed-off-by: Yinghai Lu <yinghai@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: Jesse Barnes <jbarnes@...tuousgeek.org>
Cc: Len Brown <lenb@...nel.org>
LKML-Reference: <4A01C3BB.1000609@...nel.org>
Signed-off-by: Ingo Molnar <mingo@...e.hu>
---
 arch/x86/kernel/mpparse.c |   25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

Index: tip/arch/x86/kernel/mpparse.c
===================================================================
--- tip.orig/arch/x86/kernel/mpparse.c
+++ tip/arch/x86/kernel/mpparse.c
@@ -870,24 +870,17 @@ static
 inline void __init check_irq_src(struct mpc_intsrc *m, int *nr_m_spare) {}
 #endif /* CONFIG_X86_IO_APIC */
 
-static int check_slot(unsigned long mpc_new_phys, unsigned long mpc_new_length,
-		      int count)
+static int
+check_slot(unsigned long mpc_new_phys, unsigned long mpc_new_length, int count)
 {
-	if (!mpc_new_phys) {
-		pr_info("No spare slots, try to append...take your risk, "
-			"new mpc_length %x\n", count);
-	} else {
-		if (count <= mpc_new_length)
-			pr_info("No spare slots, try to append..., "
-				"new mpc_length %x\n", count);
-		else {
-			pr_err("mpc_new_length %lx is too small\n",
-				mpc_new_length);
-			return -1;
-		}
+	int ret = 0;
+
+	if (!mpc_new_phys || count <= mpc_new_length) {
+		WARN(1, "update_mptable: No spare slots (length: %x)\n", count);
+		return -1;
 	}
 
-	return 0;
+	return ret;
 }
 
 static int  __init replace_intsrc_all(struct mpc_table *mpc,
@@ -946,7 +939,7 @@ static int  __init replace_intsrc_all(st
 		} else {
 			struct mpc_intsrc *m = (struct mpc_intsrc *)mpt;
 			count += sizeof(struct mpc_intsrc);
-			if (!check_slot(mpc_new_phys, mpc_new_length, count))
+			if (check_slot(mpc_new_phys, mpc_new_length, count) < 0)
 				goto out;
 			assign_to_mpc_intsrc(&mp_irqs[i], m);
 			mpc->length = count;
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ