Re: [patch] bug in cpuid & msr on nosmp machine

[Andrew Morton]

matthieu castet <castet.matthieu@xxxxxxx> wrote:
>
> on monocpu machine (and maybe even on smp machine), when you try to 
>  acces to a cpu that don't exist (/dev/cpu/1/cpuid), cpuid (msr) call 
>  cpu_online, but on nosmp machine if the cpu!=0 this procude a BUG();
>  So I add a check that verify if the cpu can exist before calling cpu_online.
> 
>  Matthieu CASTET
> 
> 
> [cpuid.patch  text/x-patch (589 bytes)]
>  --- linux/arch/i386/kernel/cpuid.c	2004-05-18 20:47:05.000000000 +0200
>  +++ linux-2.6.5/arch/i386/kernel/cpuid.c	2004-04-04 05:36:12.000000000 +0200
>  @@ -135,7 +135,7 @@
>     int cpu = iminor(file->f_dentry->d_inode);
>     struct cpuinfo_x86 *c = &(cpu_data)[cpu];
>   
>  -  if (cpu >= num_possible_cpus() || !cpu_online(cpu))
>  +  if (!cpu_online(cpu))
>       return -ENXIO;		/* No such CPU */
>     if ( c->cpuid_level < 0 )
>       return -EIO;		/* CPUID not supported */
> 

I think what you want here is

	if (!cpu_possible(cpu) || !cpu_online(cpu))
		return -ENXIO;

Like the below.  Rusty agree?


--- 25/arch/i386/kernel/cpuid.c~cpuid-msr-range-checking-fix	2004-05-20 00:30:21.812166544 -0700
+++ 25-akpm/arch/i386/kernel/cpuid.c	2004-05-20 00:31:16.607836336 -0700
@@ -133,10 +133,12 @@ static ssize_t cpuid_read(struct file *f
 static int cpuid_open(struct inode *inode, struct file *file)
 {
 	int cpu = iminor(file->f_dentry->d_inode);
-	struct cpuinfo_x86 *c = &(cpu_data)[cpu];
+	struct cpuinfo_x86 *c;
 
-	if (!cpu_online(cpu))
+	if (!cpu_possible(cpu) || !cpu_online(cpu))
 		return -ENXIO;	/* No such CPU */
+
+	c = &(cpu_data)[cpu];
 	if (c->cpuid_level < 0)
 		return -EIO;	/* CPUID not supported */
 
diff -puN arch/i386/kernel/msr.c~cpuid-msr-range-checking-fix arch/i386/kernel/msr.c
--- 25/arch/i386/kernel/msr.c~cpuid-msr-range-checking-fix	2004-05-20 00:30:21.836162896 -0700
+++ 25-akpm/arch/i386/kernel/msr.c	2004-05-20 00:31:56.952702984 -0700
@@ -239,10 +239,12 @@ static ssize_t msr_write(struct file * f
 static int msr_open(struct inode *inode, struct file *file)
 {
   int cpu = iminor(file->f_dentry->d_inode);
-  struct cpuinfo_x86 *c = &(cpu_data)[cpu];
-  
-  if (!cpu_online(cpu))
-    return -ENXIO;		/* No such CPU */
+  struct cpuinfo_x86 *c;
+
+  if (!cpu_possible(cpu) || !cpu_online(cpu))
+    return -ENXIO;	/* No such CPU */
+
+  c = &(cpu_data)[cpu];
   if ( !cpu_has(c, X86_FEATURE_MSR) )
     return -EIO;		/* MSR not supported */
   

_

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/