[OPERA] Potential bugs detected by static analysis tool in 2.6.4

[OPERA] Potential bugs detected by static analysis tool in 2.6.4

[Zhenmin Li]

We ran our static analysis tool upon Linux 2.6.4 source files, and found
some potential errors. Since all of them are detected by the tool, we need
more effort to inspect. We would appreciate your help if anyone can verify
whether they are bugs or not.

Thanks a lot,

OPERA Research Group
University of Illinois at Urbana-Champaign



Version: 2.6.4
Files:
/arch/sparc/prom/memory.c
/arch/sparc64/prom/memory.c
/arch/sparc/kernel/sun4m_smp.c
/arch/sparc64/kernel/sunos_ioctl32.c
/arch/x86_64/kernel/mpparse.c
/arch/mips/kernel/sysirix.c
/arch/ppc/platforms/pmac_feature.c
/arch/m68k/mac/iop.c
/drivers/pci/hotplug/shpchp_ctrl.c
/sound/oss/swarm_cs4297a.c



1. /arch/sparc/prom/memory.c, Line 158-159:
prom_prom_taken[iter].theres_more = &prom_phys_total[iter+1];

Maybe change to:
prom_prom_taken[iter].theres_more = &prom_prom_taken[iter+1];



2. /arch/sparc64/prom/memory.c, Line 116-117:
prom_prom_taken[iter].theres_more = &prom_phys_total[iter+1];

Maybe change to:
prom_prom_taken[iter].theres_more = &prom_prom_taken[iter+1];



3. /arch/sparc/kernel/sun4m_smp.c, Line 227-228:
__cpu_number_map[i] = i;
__cpu_logical_map[i] = i;

Maybe change to:
__cpu_number_map[i] = cpucount;
__cpu_logical_map[cpucount] = i;



4. /arch/sparc64/kernel/sunos_ioctl32.c, Line 163-168:
case _IOW('i', 21, struct ifreq): /* SIOCSIFMTU */
ret = sys_ioctl(fd, SIOCSIFMTU, arg);
goto out;
case _IOWR('i', 22, struct ifreq): /* SIOCGIFMTU */
ret = sys_ioctl(fd, SIOCGIFMTU, arg);
goto out;

Maybe change to:
case _IOW('i', 21, struct ifreq32): /* SIOCSIFMTU */
ret = compat_sys_ioctl(fd, SIOCSIFMTU, arg);
goto out;
case _IOWR('i', 22, struct ifreq32): /* SIOCGIFMTU */
ret = compat_sys_ioctl(fd, SIOCGIFMTU, arg);
goto out;



5. /arch/x86_64/kernel/mpparse.c, Line 652:
Dprintk("Boot CPU = %d\n", boot_cpu_physical_apicid);

Maybe change to:
Dprintk("Boot CPU = %d\n", boot_cpu_id);



6. /arch/mips/kernel/sysirix.c, Line 1643:
error = verify_area(VERIFY_WRITE, buf, sizeof(struct irix_statvfs));

Maybe change to:
error = verify_area(VERIFY_WRITE, buf, sizeof(struct irix_statvfs64));



7. /arch/ppc/platforms/pmac_feature.c, Line 1160:
MACIO_BIS(KEYLARGO_FCR0, KL1_USB2_CELL_ENABLE);

Maybe change to:
MACIO_BIS(KEYLARGO_FCR1, KL1_USB2_CELL_ENABLE);



8. /arch/m68k/mac/iop.c, Line 164:
iop_base[IOP_NUM_SCC]->status_ctrl = 0;

Maybe change to:
iop_base[IOP_NUM_ISM]->status_ctrl = 0;



9. /drivers/pci/hotplug/shpchp_ctrl.c, Line 1575:
err("%s: Failed to disable slot, error code(%d)\n", __FUNCTION__, rc);

Maybe change to:
err("%s: Failed to disable slot, error code(%d)\n", __FUNCTION__, retval);



10. /sound/oss/swarm_cs4297a.c, Line 2019:
s->dma_adc.blocks = s->dma_dac.wakeup = 0;

Maybe change to:
s->dma_adc.blocks = s->dma_adc.wakeup = 0;

Re: [OPERA] Potential bugs detected by static analysis tool in 2.6.4

[Geert Uytterhoeven]

On Wed, 5 May 2004, Zhenmin Li wrote:
> We ran our static analysis tool upon Linux 2.6.4 source files, and found
> some potential errors. Since all of them are detected by the tool, we need
> more effort to inspect. We would appreciate your help if anyone can verify
> whether they are bugs or not.
>
> Thanks a lot,
>
> OPERA Research Group
> University of Illinois at Urbana-Champaign
>
>
>
> Version: 2.6.4

    [...]

> 8. /arch/m68k/mac/iop.c, Line 164:

Should be line 264?

> iop_base[IOP_NUM_SCC]->status_ctrl = 0;
>
> Maybe change to:
> iop_base[IOP_NUM_ISM]->status_ctrl = 0;

Mac guys, is this correct?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [OPERA] Potential bugs detected by static analysis tool in 2.6.4

[Brad Boyer]

On Thu, May 06, 2004 at 11:43:45AM +0200, Geert Uytterhoeven wrote:
> On Wed, 5 May 2004, Zhenmin Li wrote:
> > 8. /arch/m68k/mac/iop.c, Line 164:
> 
> Should be line 264?

Perhaps they cut out the comments before counting lines? There are
around 100 lines of comments at the top explaining the whole mess.

In any case, line 264 sounds right. It's shortly after

if(macintosh_config->adb_type == MAC_ADB_IOP) {

> > iop_base[IOP_NUM_SCC]->status_ctrl = 0;
> >
> > Maybe change to:
> > iop_base[IOP_NUM_ISM]->status_ctrl = 0;
> 
> Mac guys, is this correct?

Actually, I think it is. It looks like this is a bug that crept in
during the last IOP rewrite (back in 2.2). It's not the same line
number in 2.2 and 2.4, but there is a similar situation. I'll see
if I can find some time to get my Mac IIfx running again and try
out a fix. I did get a 2.6 kernel running on it once before.

And as a note to the person who reported this, please include at
least a line or two of context around the change. If you use
diff -u, that's even better.

	Brad Boyer
	flar@allandria.com

RE: [OPERA] Potential bugs detected by static analysis tool in 2.6.4

[Zhenmin Li]

Sorry for the typo. The line number should be 264, and the context is:

if (macintosh_config->adb_type == MAC_ADB_IOP) {
	if (macintosh_config->ident == MAC_MODEL_IIFX) {
		iop_base[IOP_NUM_ISM] = (struct mac_iop *)
ISM_IOP_BASE_IIFX;
	} else {
		iop_base[IOP_NUM_ISM] = (struct mac_iop *)
ISM_IOP_BASE_QUADRA;
	}
	iop_base[IOP_NUM_SCC]->status_ctrl = 0;
	iop_ism_present = 1;
}

Thanks,
OPERA Research Group

 

-----Original Message-----
From: linux-kernel-owner@vger.kernel.org
[mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Geert Uytterhoeven
Sent: Thursday, May 06, 2004 4:44 AM
To: Linux/m68k; Linux/m68k on Mac
Cc: Zhenmin Li; Linux Kernel Development
Subject: Re: [OPERA] Potential bugs detected by static analysis tool in
2.6.4

On Wed, 5 May 2004, Zhenmin Li wrote:
> We ran our static analysis tool upon Linux 2.6.4 source files, and found
> some potential errors. Since all of them are detected by the tool, we need
> more effort to inspect. We would appreciate your help if anyone can verify
> whether they are bugs or not.
>
> Thanks a lot,
>
> OPERA Research Group
> University of Illinois at Urbana-Champaign
>
>
>
> Version: 2.6.4

    [...]

> 8. /arch/m68k/mac/iop.c, Line 164:

Should be line 264?

> iop_base[IOP_NUM_SCC]->status_ctrl = 0;
>
> Maybe change to:
> iop_base[IOP_NUM_ISM]->status_ctrl = 0;

Mac guys, is this correct?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like
that.
							    -- Linus
Torvalds
-
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: [OPERA] Potential bugs detected by static analysis tool in 2.6.4

[Brad Boyer]

On Thu, May 06, 2004 at 11:44:37AM -0500, Zhenmin Li wrote:
> Sorry for the typo. The line number should be 264, and the context is:
> 
> if (macintosh_config->adb_type == MAC_ADB_IOP) {
> 	if (macintosh_config->ident == MAC_MODEL_IIFX) {
> 		iop_base[IOP_NUM_ISM] = (struct mac_iop *)
> ISM_IOP_BASE_IIFX;
> 	} else {
> 		iop_base[IOP_NUM_ISM] = (struct mac_iop *)
> ISM_IOP_BASE_QUADRA;
> 	}
> 	iop_base[IOP_NUM_SCC]->status_ctrl = 0;
> 	iop_ism_present = 1;
> }

That's what I suspected. Yes, this code is broken, and line 264
should have IOP_NUM_ISM, just like the other two lines. The
current IOP driver is a bit of a hack, and I guess it just slipped
through the cracks. We rely on the chips to be mostly initialized
before we touch them as it is.

This line should be fixed as suggested, and if it breaks anything,
it's because the code was broken before, and we just didn't notice.
This driver only gets used on 3 Macintosh models, all of which are
relatively obscure.

	Brad Boyer
	flar@allandria.com

Re: [OPERA] Potential bugs detected by static analysis tool in 2.6.4

[Luiz Fernando N. Capitulino]

 Hi Zhenmin,

Em Wed, May 05, 2004 at 10:41:37PM -0500, Zhenmin Li escreveu:

| 9. /drivers/pci/hotplug/shpchp_ctrl.c, Line 1575:
| err("%s: Failed to disable slot, error code(%d)\n", __FUNCTION__, rc);
| 
| Maybe change to:
| err("%s: Failed to disable slot, error code(%d)\n", __FUNCTION__, retval);

 This seems right to me.

| 
| 10. /sound/oss/swarm_cs4297a.c, Line 2019:
| s->dma_adc.blocks = s->dma_dac.wakeup = 0;
| 
| Maybe change to:
| s->dma_adc.blocks = s->dma_adc.wakeup = 0;

 This don't. At a first look seems that 'dma_adc' and 'dma_dac' is the some
thing typed wrong, but don't, they are not the some thing.

-- 
Luiz Fernando N. Capitulino
<http://www.telecentros.sp.gov.br>

[OPERA] Another potential error detected by static analysis tool (2.6.4)

[Zhenmin Li]

Version: 2.6.4
Files:
/arch/alpha/kernel/irq.c

--- 539,547 ----
  #ifdef CONFIG_SMP
        if (i == 0) {
                seq_puts(p, "           ");
!               for (i = 0; i < NR_CPUS; i++)
!                       if (cpu_online(i))
!                               seq_printf(p, "CPU%d       ", i);
                seq_putc(p, '\n');
        }
  #endif


Maybe change to:
*** 539,547 ****
  #ifdef CONFIG_SMP
        if (i == 0) {
                seq_puts(p, "           ");
!               for (j = 0; j < NR_CPUS; j++)
!                       if (cpu_online(j))
!                               seq_printf(p, "CPU%d       ", j);
                seq_putc(p, '\n');
        }
  #endif




OPERA Research Group
University of Illinois at Urbana-Champaign

Re: [OPERA] Another potential error detected by static analysis tool (2.6.4)

[Andreas Dilger]

On May 07, 2004  17:22 -0500, Zhenmin Li wrote:
> Version: 2.6.4
> Files:
> /arch/alpha/kernel/irq.c
> 
> --- 539,547 ----
>   #ifdef CONFIG_SMP
>         if (i == 0) {
>                 seq_puts(p, "           ");
> !               for (i = 0; i < NR_CPUS; i++)
> !                       if (cpu_online(i))
> !                               seq_printf(p, "CPU%d       ", i);
>                 seq_putc(p, '\n');
>         }
>   #endif
> 
> 
> Maybe change to:
> *** 539,547 ****
>   #ifdef CONFIG_SMP
>         if (i == 0) {
>                 seq_puts(p, "           ");
> !               for (j = 0; j < NR_CPUS; j++)
> !                       if (cpu_online(j))
> !                               seq_printf(p, "CPU%d       ", j);
>                 seq_putc(p, '\n');
>         }
>   #endif

The real bug is that someone used "i" for something meaningful other than
a loop counter.  In addition to your change, this should be fixed to call
that variable "irq" or something more useful ;-).

Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/

Re: [OPERA] Potential bugs detected by static analysis tool in 2.6.4

[Andi Kleen]

"Zhenmin Li" <zli4@cs.uiuc.edu> writes:

> 5. /arch/x86_64/kernel/mpparse.c, Line 652:
> Dprintk("Boot CPU = %d\n", boot_cpu_physical_apicid);
> 
> Maybe change to:
> Dprintk("Boot CPU = %d\n", boot_cpu_id);

They are the same anyways:

include/asm-x86_64/acpi.h:144:#define boot_cpu_physical_apicid boot_cpu_id

-Andi