re: [POWERPC] Add QUICC Engine (QE) infrastructure

Hello Li Yang,

The patch 986585385131: "[POWERPC] Add QUICC Engine (QE)
infrastructure" from Oct 3, 2006, leads to the following static
checker warning:

	drivers/soc/fsl/qe/qe_ic.c:412 qe_ic_set_priority()
	error: buffer overflow 'qe_ic_info' 44 <= 127

drivers/soc/fsl/qe/qe_ic.c
   401  /* Set Priority level within its group, from 1 to 8 */
   402  int qe_ic_set_priority(unsigned int virq, unsigned int priority)
   403  {
   404          struct qe_ic *qe_ic = qe_ic_from_irq(virq);
   405          unsigned int src = virq_to_hw(virq);
   406          u32 temp;
   407  
   408          if (priority > 8 || priority == 0)
   409                  return -EINVAL;
   410          if (src > 127)

The 127 isn't right here, I'm not sure where that value comes from.  It
should be >= ARRAY_SIZE(qe_ic_info).

   411                  return -EINVAL;
   412          if (qe_ic_info[src].pri_reg == 0)
   413                  return -EINVAL;
   414
   415          temp = qe_ic_read(qe_ic->regs, qe_ic_info[src].pri_reg);
   416
   417          if (priority < 4) {
   418                  temp &= ~(0x7 << (32 - priority * 3));
   419                  temp |= qe_ic_info[src].pri_code << (32 - priority * 3);
   420          } else {
   421                  temp &= ~(0x7 << (24 - priority * 3));
   422                  temp |= qe_ic_info[src].pri_code << (24 - priority * 3);
   423          }
   424
   425          qe_ic_write(qe_ic->regs, qe_ic_info[src].pri_reg, temp);
   426
   427          return 0;
   428  }

regards,
dan carpenter

On Wed, Jan 6, 2016 at 5:27 PM, Dan Carpenter [off-list ref] wrote:

Hello Li Yang,

The patch 986585385131: "[POWERPC] Add QUICC Engine (QE)
infrastructure" from Oct 3, 2006, leads to the following static
checker warning:

        drivers/soc/fsl/qe/qe_ic.c:412 qe_ic_set_priority()
        error: buffer overflow 'qe_ic_info' 44 <= 127

drivers/soc/fsl/qe/qe_ic.c
   401  /* Set Priority level within its group, from 1 to 8 */
   402  int qe_ic_set_priority(unsigned int virq, unsigned int priority)
   403  {
   404          struct qe_ic *qe_ic = qe_ic_from_irq(virq);
   405          unsigned int src = virq_to_hw(virq);
   406          u32 temp;
   407
   408          if (priority > 8 || priority == 0)
   409                  return -EINVAL;
   410          if (src > 127)

The 127 isn't right here, I'm not sure where that value comes from.  It
should be >= ARRAY_SIZE(qe_ic_info).

Hi Dan,

Thanks for pointing this out.  127 should be the theoretical up
boundary of QEIC. You are right that we should have a stricter check.
And maybe a more serious issue is that we didn't check the overflow at
the host_map() stage.

Regards,
Leo