[bug report] Linux-2.6.12-rc2

[bug report] Linux-2.6.12-rc2

[Dan Carpenter]

Hi PPC Devs,

The patch 1da177e4c3f4: "Linux-2.6.12-rc2" from Apr 16, 2005, leads
to the following static checker warning:

	arch/powerpc/sysdev/ipic.c:783 ipic_set_priority()
	error: buffer overflow 'ipic_info' 95 <= 127

arch/powerpc/sysdev/ipic.c
    36  static struct ipic_info ipic_info[] = {
    37          [1] = {
    38                  .mask   = IPIC_SIMSR_H,
    39                  .prio   = IPIC_SIPRR_C,
    40                  .force  = IPIC_SIFCR_H,
    41                  .bit    = 16,
    42                  .prio_mask = 0,
    43          },

 [ huge 95 element array snipped ]

   500          [94] = {
   501                  .mask   = IPIC_SIMSR_L,
   502                  .prio   = 0,
   503                  .force  = IPIC_SIFCR_L,
   504                  .bit    = 30,
   505          },
   506  };

 [ more code snipped ]

   773  int ipic_set_priority(unsigned int virq, unsigned int priority)
   774  {
   775          struct ipic *ipic = ipic_from_irq(virq);
   776          unsigned int src = virq_to_hw(virq);
   777          u32 temp;
   778  
   779          if (priority > 7)
   780                  return -EINVAL;
   781          if (src > 127)
                    ^^^^^^^^^
We cap this at 127

   782                  return -EINVAL;
   783          if (ipic_info[src].prio == 0)
                    ^^^^^^^^^^^^^^
But we only have 95 elements.  Should the array be larger or should
we >= ARRAY_SIZE(ipic_info) is invalid?

   784                  return -EINVAL;
   785  

regards,
dan carpenter

Re: [bug report] Linux-2.6.12-rc2

[Michael Ellerman]

Dan Carpenter <dan.carpenter@oracle.com> writes:

> Hi PPC Devs,
>
> The patch 1da177e4c3f4: "Linux-2.6.12-rc2" from Apr 16, 2005, leads

Might want to special case that one :)

> to the following static checker warning:
>
> 	arch/powerpc/sysdev/ipic.c:783 ipic_set_priority()
> 	error: buffer overflow 'ipic_info' 95 <= 127
...
>
>    773  int ipic_set_priority(unsigned int virq, unsigned int priority)
>    774  {
>    775          struct ipic *ipic = ipic_from_irq(virq);
>    776          unsigned int src = virq_to_hw(virq);
>    777          u32 temp;
>    778  
>    779          if (priority > 7)
>    780                  return -EINVAL;
>    781          if (src > 127)
>                     ^^^^^^^^^
> We cap this at 127
>
>    782                  return -EINVAL;
>    783          if (ipic_info[src].prio == 0)
>                     ^^^^^^^^^^^^^^
> But we only have 95 elements.  Should the array be larger or should
> we >= ARRAY_SIZE(ipic_info) is invalid?

I don't know the code personally, but looking at the history it seems new
interrupts are added manually with specific flags.

So testing against ARRAY_SIZE would be the best fix AFAICS.

cheers

[bug report] Linux-2.6.12-rc2

[Dan Carpenter]

Hey,

This code is older than git is so it probably doesn't matter.  But just
for laughs does anyone know what this should be?

	drivers/scsi/aic7xxx/aic7xxx_core.c:4807 ahc_init_scbdata()
	warn: integer overflow (literal): u32max + 1

drivers/scsi/aic7xxx/aic7xxx_core.c
  4794  
  4795          /*
  4796           * Create our DMA tags.  These tags define the kinds of device
  4797           * accessible memory allocations and memory mappings we will
  4798           * need to perform during normal operation.
  4799           *
  4800           * Unless we need to further restrict the allocation, we rely
  4801           * on the restrictions of the parent dmat, hence the common
  4802           * use of MAXADDR and MAXSIZE.
  4803           */
  4804  
  4805          /* DMA tag for our hardware scb structures */
  4806          if (ahc_dma_tag_create(ahc, ahc->parent_dmat, /*alignment*/1,
  4807                                 /*boundary*/BUS_SPACE_MAXADDR_32BIT + 1,
                                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^
This is "0xffffffff + 1" which has an integer overflow so it's a
complicated way to say zero.

  4808                                 /*lowaddr*/BUS_SPACE_MAXADDR_32BIT,
  4809                                 /*highaddr*/BUS_SPACE_MAXADDR,
  4810                                 /*filter*/NULL, /*filterarg*/NULL,
  4811                                 AHC_SCB_MAX_ALLOC * sizeof(struct hardware_scb),
  4812                                 /*nsegments*/1,
  4813                                 /*maxsegsz*/BUS_SPACE_MAXSIZE_32BIT,
  4814                                 /*flags*/0, &scb_data->hscb_dmat) != 0) {
  4815                  goto error_exit;
  4816          }
  4817  
  4818          scb_data->init_level++;
  4819  

regards,
dan carpenter