[patch 0/8] -stable review

[patch 0/8] -stable review

[Chris Wright]

This is the start of the stable review cycle for the 2.6.12.5 release.
There are 8 patches in this series, all will be posted as a response to
this one.  If anyone has any issues with these being applied, please let
us know.  If anyone is a maintainer of the proper subsystem, and wants
to add a signed-off-by: line to the patch, please respond with it.

These patches are sent out with a number of different people on the
Cc: line.  If you wish to be a reviewer, please email stable@kernel.org
to add your name to the list.  If you want to be off the reviewer list,
also email us.

Responses should be made by Sat, Aug 13, 22:00 UTC.  Anything received
after that time, might be too late.

thanks,

the -stable release team
--

[patch 1/8] [PATCH] sys_set_mempolicy() doesnt check if mode < 0

[Chris Wright]

[-- Attachment #1: sys_set_mempolicy-mode-check.patch --]
[-- Type: text/plain, Size: 1098 bytes --]

-stable review patch.  If anyone has any  objections, please let us know.
------------------


A kernel BUG() is triggered by a call to set_mempolicy() with a negative
first argument.  This is because the mode is declared as an int, and the
validity check doesnt check < 0 values.  Alternatively, mode could be
declared as unsigned int or unsigned long.

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
Cc: Andi Kleen <ak@suse.de>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Signed-off-by: Chris Wright <chrisw@osdl.org>
---
 mm/mempolicy.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.12.y/mm/mempolicy.c
===================================================================
--- linux-2.6.12.y.orig/mm/mempolicy.c
+++ linux-2.6.12.y/mm/mempolicy.c
@@ -409,7 +409,7 @@ asmlinkage long sys_set_mempolicy(int mo
 	struct mempolicy *new;
 	DECLARE_BITMAP(nodes, MAX_NUMNODES);
 
-	if (mode > MPOL_MAX)
+	if (mode < 0 || mode > MPOL_MAX)
 		return -EINVAL;
 	err = get_nodes(nodes, nmask, maxnode, mode);
 	if (err)

--

[patch 2/8] [PATCH] Fix SRAT for non dual core AMD systems

[Chris Wright]

[-- Attachment #1: x86_64-srat-dual-core-amd.patch --]
[-- Type: text/plain, Size: 787 bytes --]

-stable review patch.  If anyone has any  objections, please let us know.
------------------


This fixes a bug in SRAT handling on AMD systems that was introduced
with the dual core support. It would be disabled on CPUs without dual core.
Just drop the bogus check.

Signed-off-by: Andi Kleen <ak@suse.de>
Signed-off-by: Chris Wright <chrisw@osdl.org>

Index: linux-2.6.12/arch/x86_64/kernel/setup.c
===================================================================
--- linux-2.6.12.orig/arch/x86_64/kernel/setup.c
+++ linux-2.6.12/arch/x86_64/kernel/setup.c
@@ -729,8 +729,6 @@ static void __init amd_detect_cmp(struct
 	int cpu = smp_processor_id();
 	int node = 0;
 	unsigned bits;
-	if (c->x86_num_cores == 1)
-		return;
 
 	bits = 0;
 	while ((1 << bits) < c->x86_num_cores)

--

[patch 3/8] [PATCH] x86_64: Fixing smpboot timing problem

[Chris Wright]

[-- Attachment #1: x86_64-fix-smpboot-timing-problem.patch --]
[-- Type: text/plain, Size: 5559 bytes --]

-stable review patch.  If anyone has any  objections, please let us know.
------------------

introduced in 2.6.12. Please apply to stable.

>From Eric Biederman

sync_tsc was using smp_call_function to ask the boot processor
to report it's tsc value.  smp_call_function performs an IPI_send_allbutself
which is a broadcast ipi.  There is a window during processor startup during
which the target cpu has started and before it has initialized it's interrupt
vectors so it can properly process an interrupt.  Receveing an interrupt
during that window will triple fault the cpu and do other nasty things.

Why cli does not protect us from that is beyond me.

The simple fix is to match ia64 and provide a smp_call_function_single.
Which avoids the broadcast and is more efficient.

This certainly fixes the problem of getting stuck on boot which was
very easy to trigger on my SMP Hyperthreaded Xeon, and I think
it fixes it for the right reasons.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Andi Kleen <ak@suse.de>

Index: linux-2.6.12/arch/x86_64/kernel/smp.c
===================================================================
--- linux-2.6.12.orig/arch/x86_64/kernel/smp.c
+++ linux-2.6.12/arch/x86_64/kernel/smp.c
@@ -284,6 +284,71 @@ struct call_data_struct {
 static struct call_data_struct * call_data;
 
 /*
+ * this function sends a 'generic call function' IPI to one other CPU
+ * in the system.
+ */
+static void __smp_call_function_single (int cpu, void (*func) (void *info), void *info,
+				int nonatomic, int wait)
+{
+	struct call_data_struct data;
+	int cpus = 1;
+
+	data.func = func;
+	data.info = info;
+	atomic_set(&data.started, 0);
+	data.wait = wait;
+	if (wait)
+		atomic_set(&data.finished, 0);
+
+	call_data = &data;
+	wmb();
+	/* Send a message to all other CPUs and wait for them to respond */
+	send_IPI_mask(cpumask_of_cpu(cpu), CALL_FUNCTION_VECTOR);
+
+	/* Wait for response */
+	while (atomic_read(&data.started) != cpus)
+		cpu_relax();
+
+	if (!wait)
+		return;
+
+	while (atomic_read(&data.finished) != cpus)
+		cpu_relax();
+}
+
+/*
+ * Run a function on another CPU
+ *  <func>	The function to run. This must be fast and non-blocking.
+ *  <info>	An arbitrary pointer to pass to the function.
+ *  <nonatomic>	Currently unused.
+ *  <wait>	If true, wait until function has completed on other CPUs.
+ *  [RETURNS]   0 on success, else a negative status code.
+ *
+ * Does not return until the remote CPU is nearly ready to execute <func>
+ * or is or has executed.
+ */
+
+int smp_call_function_single (int cpu, void (*func) (void *info), void *info, 
+	int nonatomic, int wait)
+{
+	
+	int me = get_cpu(); /* prevent preemption and reschedule on another processor */
+
+	if (cpu == me) {
+		printk("%s: trying to call self\n", __func__);
+		put_cpu();
+		return -EBUSY;
+	}
+	spin_lock_bh(&call_lock);
+
+	__smp_call_function_single(cpu, func,info,nonatomic,wait);	
+
+	spin_unlock_bh(&call_lock);
+	put_cpu();
+	return 0;
+}
+
+/*
  * this function sends a 'generic call function' IPI to all other CPUs
  * in the system.
  */
Index: linux-2.6.12/arch/x86_64/kernel/smpboot.c
===================================================================
--- linux-2.6.12.orig/arch/x86_64/kernel/smpboot.c
+++ linux-2.6.12/arch/x86_64/kernel/smpboot.c
@@ -202,9 +202,6 @@ static __cpuinit void sync_master(void *
 {
 	unsigned long flags, i;
 
-	if (smp_processor_id() != boot_cpu_id)
-		return;
-
 	go[MASTER] = 0;
 
 	local_irq_save(flags);
@@ -253,7 +250,7 @@ get_delta(long *rt, long *master)
 	return tcenter - best_tm;
 }
 
-static __cpuinit void sync_tsc(void)
+static __cpuinit void sync_tsc(unsigned int master)
 {
 	int i, done = 0;
 	long delta, adj, adjust_latency = 0;
@@ -267,9 +264,17 @@ static __cpuinit void sync_tsc(void)
 	} t[NUM_ROUNDS] __cpuinitdata;
 #endif
 
+	printk(KERN_INFO "CPU %d: Syncing TSC to CPU %u.\n",
+		smp_processor_id(), master);
+
 	go[MASTER] = 1;
 
-	smp_call_function(sync_master, NULL, 1, 0);
+	/* It is dangerous to broadcast IPI as cpus are coming up,
+	 * as they may not be ready to accept them.  So since
+	 * we only need to send the ipi to the boot cpu direct
+	 * the message, and avoid the race.
+	 */
+	smp_call_function_single(master, sync_master, NULL, 1, 0);
 
 	while (go[MASTER])	/* wait for master to be ready */
 		no_cpu_relax();
@@ -313,16 +318,14 @@ static __cpuinit void sync_tsc(void)
 	printk(KERN_INFO
 	       "CPU %d: synchronized TSC with CPU %u (last diff %ld cycles, "
 	       "maxerr %lu cycles)\n",
-	       smp_processor_id(), boot_cpu_id, delta, rt);
+	       smp_processor_id(), master, delta, rt);
 }
 
 static void __cpuinit tsc_sync_wait(void)
 {
 	if (notscsync || !cpu_has_tsc)
 		return;
-	printk(KERN_INFO "CPU %d: Syncing TSC to CPU %u.\n", smp_processor_id(),
-			boot_cpu_id);
-	sync_tsc();
+	sync_tsc(boot_cpu_id);
 }
 
 static __init int notscsync_setup(char *s)
Index: linux-2.6.12/include/asm-x86_64/smp.h
===================================================================
--- linux-2.6.12.orig/include/asm-x86_64/smp.h
+++ linux-2.6.12/include/asm-x86_64/smp.h
@@ -46,6 +46,8 @@ extern int pic_mode;
 extern int smp_num_siblings;
 extern void smp_flush_tlb(void);
 extern void smp_message_irq(int cpl, void *dev_id, struct pt_regs *regs);
+extern int smp_call_function_single (int cpuid, void (*func) (void *info), void *info,
+				     int retry, int wait);
 extern void smp_send_reschedule(int cpu);
 extern void smp_invalidate_rcv(void);		/* Process an NMI */
 extern void zap_low_mappings(void);

--

Re: [patch 3/8] [PATCH] x86_64: Fixing smpboot timing problem

[Andi Kleen]

>  static void __cpuinit tsc_sync_wait(void)
>  {
>  	if (notscsync || !cpu_has_tsc)
>  		return;
> -	printk(KERN_INFO "CPU %d: Syncing TSC to CPU %u.\n", smp_processor_id(),
> -			boot_cpu_id);
> -	sync_tsc();
> +	sync_tsc(boot_cpu_id);

I actually found a bug in this today. This should be sync_tsc(0), not sync_tsc(boot_cpu_id)
Can you just fix it in your tree or should I submit a new patch? 

-Andi

Re: [patch 3/8] [PATCH] x86_64: Fixing smpboot timing problem

[Chris Wright]

* Andi Kleen (ak@suse.de) wrote:
> >  static void __cpuinit tsc_sync_wait(void)
> >  {
> >  	if (notscsync || !cpu_has_tsc)
> >  		return;
> > -	printk(KERN_INFO "CPU %d: Syncing TSC to CPU %u.\n", smp_processor_id(),
> > -			boot_cpu_id);
> > -	sync_tsc();
> > +	sync_tsc(boot_cpu_id);
> 
> I actually found a bug in this today. This should be sync_tsc(0), not sync_tsc(boot_cpu_id)
> Can you just fix it in your tree or should I submit a new patch? 

I'll fix it locally.  Thanks for the heads-up.
-chris

Re: [patch 3/8] [PATCH] x86_64: Fixing smpboot timing problem

[Eric W. Biederman]

Chris Wright <chrisw@osdl.org> writes:

> * Andi Kleen (ak@suse.de) wrote:
>> >  static void __cpuinit tsc_sync_wait(void)
>> >  {
>> >  	if (notscsync || !cpu_has_tsc)
>> >  		return;
>> > - printk(KERN_INFO "CPU %d: Syncing TSC to CPU %u.\n", smp_processor_id(),
>> > -			boot_cpu_id);
>> > -	sync_tsc();
>> > +	sync_tsc(boot_cpu_id);
>> 
>> I actually found a bug in this today. This should be sync_tsc(0), not
> sync_tsc(boot_cpu_id)
>> Can you just fix it in your tree or should I submit a new patch? 
>
> I'll fix it locally.  Thanks for the heads-up.

Someone needs to send the patch to Linus for 2.6.13 as well.  Is someone
else going to or should I.  I knew I was confused about physical versus
logical apic ids when I generated the patch.

Eric

[PATCH] x86_64: Fix apicid versus cpu# confusion.

[Eric W. Biederman]

Ok being impatient not wanting this tiny bug to be forgotten for
2.6.13.  Linus please apply this micro patch.

> >  static void __cpuinit tsc_sync_wait(void)
> >  {
> >  	if (notscsync || !cpu_has_tsc)
> >  		return;
> > - printk(KERN_INFO "CPU %d: Syncing TSC to CPU %u.\n", smp_processor_id(),
> > -			boot_cpu_id);
> > -	sync_tsc();
> > +	sync_tsc(boot_cpu_id);
>
> I actually found a bug in this today. This should be sync_tsc(0), not
> sync_tsc(boot_cpu_id)
> Can you just fix it in your tree or should I submit a new patch?
>
> -Andi

Oops.  I knew I didn't have the physical versus logical cpu identifiers right
when I generated that patch.  It's not nearly as bad as I feared at the time
though.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 arch/x86_64/kernel/smpboot.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

5192895f71c7eff7e1335cd9d6a775dda2bb5d52
diff --git a/arch/x86_64/kernel/smpboot.c b/arch/x86_64/kernel/smpboot.c
--- a/arch/x86_64/kernel/smpboot.c
+++ b/arch/x86_64/kernel/smpboot.c
@@ -334,7 +334,7 @@ static void __cpuinit tsc_sync_wait(void
 {
 	if (notscsync || !cpu_has_tsc)
 		return;
-	sync_tsc(boot_cpu_id);
+	sync_tsc(0);
 }
 
 static __init int notscsync_setup(char *s)

Re: [PATCH] x86_64: Fix apicid versus cpu# confusion.

[yhlu]

So boot_cpu_id is apic id of BSP.

Anyway sync_tsc(...) there is master and MASTER..., but value are all 0.

YH

On 8/11/05, Eric W. Biederman <ebiederm@xmission.com> wrote:
> Ok being impatient not wanting this tiny bug to be forgotten for
> 2.6.13.  Linus please apply this micro patch.
> 
> > >  static void __cpuinit tsc_sync_wait(void)
> > >  {
> > >     if (notscsync || !cpu_has_tsc)
> > >             return;
> > > - printk(KERN_INFO "CPU %d: Syncing TSC to CPU %u.\n", smp_processor_id(),
> > > -                   boot_cpu_id);
> > > -   sync_tsc();
> > > +   sync_tsc(boot_cpu_id);
> >
> > I actually found a bug in this today. This should be sync_tsc(0), not
> > sync_tsc(boot_cpu_id)
> > Can you just fix it in your tree or should I submit a new patch?
> >
> > -Andi
> 
> Oops.  I knew I didn't have the physical versus logical cpu identifiers right
> when I generated that patch.  It's not nearly as bad as I feared at the time
> though.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
> 
>  arch/x86_64/kernel/smpboot.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> 5192895f71c7eff7e1335cd9d6a775dda2bb5d52
> diff --git a/arch/x86_64/kernel/smpboot.c b/arch/x86_64/kernel/smpboot.c
> --- a/arch/x86_64/kernel/smpboot.c
> +++ b/arch/x86_64/kernel/smpboot.c
> @@ -334,7 +334,7 @@ static void __cpuinit tsc_sync_wait(void
>  {
>         if (notscsync || !cpu_has_tsc)
>                 return;
> -       sync_tsc(boot_cpu_id);
> +       sync_tsc(0);
>  }
> 
>  static __init int notscsync_setup(char *s)
> -
> 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/
>

[patch 4/8] [PATCH] Update in-kernel zlib routines

[Chris Wright]

[-- Attachment #1: linux-zlib-fixes.patch --]
[-- Type: text/plain, Size: 2793 bytes --]

-stable review patch.  If anyone has any  objections, please let us know.
------------------


a) http://sources.redhat.com/ml/bug-gnu-utils/1999-06/msg00183.html
b) http://bugs.gentoo.org/show_bug.cgi?id=94584

Signed-off-by: Tim Yamin <plasmaroo@gentoo.org>
Signed-off-by: Tavis Ormandy <taviso@gentoo.org>
Signed-off-by: Chris Wright <chrisw@osdl.org>
---
 arch/ppc64/boot/zlib.c      |    3 ++-
 lib/inflate.c               |   16 +++++++++-------
 lib/zlib_inflate/inftrees.c |    2 +-
 3 files changed, 12 insertions(+), 9 deletions(-)

Index: linux-2.6.12.y/lib/inflate.c
===================================================================
--- linux-2.6.12.y.orig/lib/inflate.c
+++ linux-2.6.12.y/lib/inflate.c
@@ -326,7 +326,7 @@ DEBG("huft1 ");
   {
     *t = (struct huft *)NULL;
     *m = 0;
-    return 0;
+    return 2;
   }
 
 DEBG("huft2 ");
@@ -374,6 +374,7 @@ DEBG("huft5 ");
     if ((j = *p++) != 0)
       v[x[j]++] = i;
   } while (++i < n);
+  n = x[g];                   /* set n to length of v */
 
 DEBG("h6 ");
 
@@ -410,12 +411,13 @@ DEBG1("1 ");
 DEBG1("2 ");
           f -= a + 1;           /* deduct codes from patterns left */
           xp = c + k;
-          while (++j < z)       /* try smaller tables up to z bits */
-          {
-            if ((f <<= 1) <= *++xp)
-              break;            /* enough codes to use up j bits */
-            f -= *xp;           /* else deduct codes from patterns */
-          }
+          if (j < z)
+            while (++j < z)       /* try smaller tables up to z bits */
+            {
+              if ((f <<= 1) <= *++xp)
+                break;            /* enough codes to use up j bits */
+              f -= *xp;           /* else deduct codes from patterns */
+            }
         }
 DEBG1("3 ");
         z = 1 << j;             /* table entries for j-bit table */
Index: linux-2.6.12.y/lib/zlib_inflate/inftrees.c
===================================================================
--- linux-2.6.12.y.orig/lib/zlib_inflate/inftrees.c
+++ linux-2.6.12.y/lib/zlib_inflate/inftrees.c
@@ -141,7 +141,7 @@ static int huft_build(
   {
     *t = NULL;
     *m = 0;
-    return Z_OK;
+    return Z_DATA_ERROR;
   }
 
 
Index: linux-2.6.12.y/arch/ppc64/boot/zlib.c
===================================================================
--- linux-2.6.12.y.orig/arch/ppc64/boot/zlib.c
+++ linux-2.6.12.y/arch/ppc64/boot/zlib.c
@@ -1307,7 +1307,7 @@ local int huft_build(
   {
     *t = (inflate_huft *)Z_NULL;
     *m = 0;
-    return Z_OK;
+    return Z_DATA_ERROR;
   }
 
 
@@ -1351,6 +1351,7 @@ local int huft_build(
     if ((j = *p++) != 0)
       v[x[j]++] = i;
   } while (++i < n);
+  n = x[g];			/* set n to length of v */
 
 
   /* Generate the Huffman codes and for each, make the table entries */

--

Re: [patch 4/8] [PATCH] Update in-kernel zlib routines

[Peter Osterlund]

Chris Wright <chrisw@osdl.org> writes:

> -stable review patch.  If anyone has any  objections, please let us know.
> ------------------
> 
> a) http://sources.redhat.com/ml/bug-gnu-utils/1999-06/msg00183.html

Why does this 6 year old bug have to be fixed in the 2.6.12 stable
series? Doesn't the patch violate this stable series rule?

 - It must fix a real bug that bothers people (not a, "This could be a
   problem..." type thing.)

Maybe the motivation was just missing from the patch description?

-- 
Peter Osterlund - petero2@telia.com
http://web.telia.com/~u89404340

Re: [patch 4/8] [PATCH] Update in-kernel zlib routines

[Chris Wright]

* Peter Osterlund (petero2@telia.com) wrote:
> Chris Wright <chrisw@osdl.org> writes:
> > a) http://sources.redhat.com/ml/bug-gnu-utils/1999-06/msg00183.html
> 
> Why does this 6 year old bug have to be fixed in the 2.6.12 stable
> series? Doesn't the patch violate this stable series rule?
> 
>  - It must fix a real bug that bothers people (not a, "This could be a
>    problem..." type thing.)
> 
> Maybe the motivation was just missing from the patch description?

These can manifest as possible overflow (1st one, given CAN-2005-2458),
or NULL deref (2nd one given CAN-2005-2459), which could have possible
security consequences.

thanks,
-chris

[patch 5/8] Check input buffer size in zisofs

[Chris Wright]

[-- Attachment #1: zisofs.patch --]
[-- Type: text/plain, Size: 1928 bytes --]

-stable review patch.  If anyone has any  objections, please let us know.
------------------


It's not the real deflateBound() in newer zlib libraries, partly because
the upcoming usage of it won't have the "stream" available, so we can't
have the same interfaces anyway.

This uses the new deflateBound() thing to sanity-check the input to the
zlib decompressor before we even bother to start reading in the blocks.

Problem noted by Tim Yamin <plasmaroo@gentoo.org>

Signed-off-by: Chris Wright <chrisw@osdl.org>
---
 fs/isofs/compress.c  |    6 ++++++
 include/linux/zlib.h |    5 +++++
 2 files changed, 11 insertions(+)

Index: linux-2.6.12.y/include/linux/zlib.h
===================================================================
--- linux-2.6.12.y.orig/include/linux/zlib.h
+++ linux-2.6.12.y/include/linux/zlib.h
@@ -506,6 +506,11 @@ extern int zlib_deflateReset (z_streamp 
    stream state was inconsistent (such as zalloc or state being NULL).
 */
 
+static inline unsigned long deflateBound(unsigned long s)
+{
+	return s + ((s + 7) >> 3) + ((s + 63) >> 6) + 11;
+}
+
 extern int zlib_deflateParams (z_streamp strm, int level, int strategy);
 /*
      Dynamically update the compression level and compression strategy.  The
Index: linux-2.6.12.y/fs/isofs/compress.c
===================================================================
--- linux-2.6.12.y.orig/fs/isofs/compress.c
+++ linux-2.6.12.y/fs/isofs/compress.c
@@ -129,8 +129,14 @@ static int zisofs_readpage(struct file *
 	cend = le32_to_cpu(*(__le32 *)(bh->b_data + (blockendptr & bufmask)));
 	brelse(bh);
 
+	if (cstart > cend)
+		goto eio;
+		
 	csize = cend-cstart;
 
+	if (csize > deflateBound(1UL << zisofs_block_shift))
+		goto eio;
+
 	/* Now page[] contains an array of pages, any of which can be NULL,
 	   and the locks on which we hold.  We should now read the data and
 	   release the pages.  If the pages are NULL the decompressed data

--

Re: [patch 5/8] Check input buffer size in zisofs

[H. Peter Anvin]

Chris Wright wrote:
> -stable review patch.  If anyone has any  objections, please let us know.

Looks good to me.  Feel free to add:

Signed-off-by: H. Peter Anvin <hpa@zytor.com>

Re: [patch 5/8] Check input buffer size in zisofs

[Chris Wright]

* H. Peter Anvin (hpa@zytor.com) wrote:
> Chris Wright wrote:
> >-stable review patch.  If anyone has any  objections, please let us know.
> 
> Looks good to me.  Feel free to add:
> 
> Signed-off-by: H. Peter Anvin <hpa@zytor.com>

Will do, thanks.
-chris

[patch 6/8] CAN-2005-2098 Error during attempt to join key management session can leave semaphore pinned

[Chris Wright]

[-- Attachment #1: key-session-join.patch --]
[-- Type: text/plain, Size: 1384 bytes --]

-stable review patch.  If anyone has any  objections, please let us know.
------------------

from hanging future joins in the D state [CAN-2005-2098].

The problem is that the error handling path for the KEYCTL_JOIN_SESSION_KEYRING
operation has one error path that doesn't release the session management
semaphore. Further attempts to get the semaphore will then sleep for ever in
the D state.

This can happen in four situations, all involving an attempt to allocate a new
session keyring:

 (1) ENOMEM.

 (2) The users key quota being reached.

 (3) A keyring name that is an empty string.

 (4) A keyring name that is too long.

Any user may attempt this operation, and so any user can cause the problem to
occur.

Signed-Off-By: David Howells <dhowells@redhat.com>
Signed-off-by: Chris Wright <chrisw@osdl.org>
---
 security/keys/process_keys.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.12.y/security/keys/process_keys.c
===================================================================
--- linux-2.6.12.y.orig/security/keys/process_keys.c
+++ linux-2.6.12.y/security/keys/process_keys.c
@@ -641,7 +641,7 @@ long join_session_keyring(const char *na
 		keyring = keyring_alloc(name, tsk->uid, tsk->gid, 0, NULL);
 		if (IS_ERR(keyring)) {
 			ret = PTR_ERR(keyring);
-			goto error;
+			goto error2;
 		}
 	}
 	else if (IS_ERR(keyring)) {

--

[patch 7/8] CAN-2005-2099 Destruction of failed keyring oopses

[Chris Wright]

[-- Attachment #1: failed-keyring-oops.patch --]
[-- Type: text/plain, Size: 1592 bytes --]

-stable review patch.  If anyone has any  objections, please let us know.
------------------

properly is destroyed without oopsing [CAN-2005-2099].

The problem occurs in three stages:

 (1) The key allocator initialises the type-specific data to all zeroes. In
     the case of a keyring, this will become a link in the keyring name list
     when the keyring is instantiated.

 (2) If a user (any user) attempts to add a keyring with anything other than
     an empty payload, the keyring instantiation function will fail with an
     error and won't add the keyring to the name list.

 (3) The keyring's destructor then sees that the keyring has a description
     (name) and tries to remove the keyring from the name list, which oopses
     because the link pointers are both zero.

This bug permits any user to take down a box trivially.

Signed-Off-By: David Howells <dhowells@redhat.com>
Signed-off-by: Chris Wright <chrisw@osdl.org>
---
 security/keys/keyring.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletion(-)

Index: linux-2.6.12.y/security/keys/keyring.c
===================================================================
--- linux-2.6.12.y.orig/security/keys/keyring.c
+++ linux-2.6.12.y/security/keys/keyring.c
@@ -188,7 +188,11 @@ static void keyring_destroy(struct key *
 
 	if (keyring->description) {
 		write_lock(&keyring_name_lock);
-		list_del(&keyring->type_data.link);
+
+		if (keyring->type_data.link.next != NULL &&
+		    !list_empty(&keyring->type_data.link))
+			list_del(&keyring->type_data.link);
+
 		write_unlock(&keyring_name_lock);
 	}
 

--

[patch 8/8] [PATCH] Module per-cpu alignment cannot always be met

[Chris Wright]

[-- Attachment #1: module-per-cpu-alignment-fix.patch --]
[-- Type: text/plain, Size: 2429 bytes --]

-stable review patch.  If anyone has any  objections, please let us know.
------------------


The module code assumes noone will ever ask for a per-cpu area more than
SMP_CACHE_BYTES aligned.  However, as these cases show, gcc asks sometimes
asks for 32-byte alignment for the per-cpu section on a module, and if
CONFIG_X86_L1_CACHE_SHIFT is 4, we hit that BUG_ON().  This is obviously an
unusual combination, as there have been few reports, but better to warn
than die.

See:
	http://www.ussg.iu.edu/hypermail/linux/kernel/0409.0/0768.html
  
And more recently:
	http://bugs.gentoo.org/show_bug.cgi?id=97006
  
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Signed-off-by: Chris Wright <chrisw@osdl.org>
---
 kernel/module.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

Index: linux-2.6.12.y/kernel/module.c
===================================================================
--- linux-2.6.12.y.orig/kernel/module.c
+++ linux-2.6.12.y/kernel/module.c
@@ -249,13 +249,18 @@ static inline unsigned int block_size(in
 /* Created by linker magic */
 extern char __per_cpu_start[], __per_cpu_end[];
 
-static void *percpu_modalloc(unsigned long size, unsigned long align)
+static void *percpu_modalloc(unsigned long size, unsigned long align,
+			     const char *name)
 {
 	unsigned long extra;
 	unsigned int i;
 	void *ptr;
 
-	BUG_ON(align > SMP_CACHE_BYTES);
+	if (align > SMP_CACHE_BYTES) {
+		printk(KERN_WARNING "%s: per-cpu alignment %li > %i\n",
+		       name, align, SMP_CACHE_BYTES);
+		align = SMP_CACHE_BYTES;
+	}
 
 	ptr = __per_cpu_start;
 	for (i = 0; i < pcpu_num_used; ptr += block_size(pcpu_size[i]), i++) {
@@ -347,7 +352,8 @@ static int percpu_modinit(void)
 }	
 __initcall(percpu_modinit);
 #else /* ... !CONFIG_SMP */
-static inline void *percpu_modalloc(unsigned long size, unsigned long align)
+static inline void *percpu_modalloc(unsigned long size, unsigned long align,
+				    const char *name)
 {
 	return NULL;
 }
@@ -1554,7 +1560,8 @@ static struct module *load_module(void _
 	if (pcpuindex) {
 		/* We have a special allocation for this section. */
 		percpu = percpu_modalloc(sechdrs[pcpuindex].sh_size,
-					 sechdrs[pcpuindex].sh_addralign);
+					 sechdrs[pcpuindex].sh_addralign,
+					 mod->name);
 		if (!percpu) {
 			err = -ENOMEM;
 			goto free_mod;

--

Re: [patch 8/8] [PATCH] Module per-cpu alignment cannot always be met

[Denis Vlasenko]

On Friday 12 August 2005 01:54, Chris Wright wrote:
> -stable review patch.  If anyone has any  objections, please let us know.
> ------------------
> 
> The module code assumes noone will ever ask for a per-cpu area more than
> SMP_CACHE_BYTES aligned.  However, as these cases show, gcc asks sometimes
> asks for 32-byte alignment for the per-cpu section on a module, and if
> CONFIG_X86_L1_CACHE_SHIFT is 4, we hit that BUG_ON().  This is obviously an
> unusual combination, as there have been few reports, but better to warn
> than die.

gcc gets increasingly sadistic about alignment:

"char global_var[] = "larger than 32 bytes"; uses silly amounts of alignment even with -Os"
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22158

Please, everybody who thinks that _32_ _byte_ alignment
is outright silly, add your comments to this bugzilla entry.
--
vda