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

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

[Dan Carpenter]

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

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

[Li Yang]

On Wed, Jan 6, 2016 at 5:27 PM, Dan Carpenter <dan.carpenter@oracle.com> 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