[bug report] misc: xilinx_sdfec: Add ability to configure LDPC

From: Dan Carpenter <dan.carpenter@oracle.com>
To: dragan.cvetic@xilinx.com
Cc: linux-arm-kernel@lists.infradead.org
Subject: [bug report] misc: xilinx_sdfec: Add ability to configure LDPC
Date: Wed, 21 Aug 2019 10:13:02 +0300	[thread overview]
Message-ID: <20190821071302.GF26957@mwanda> (raw)

Hello Dragan Cvetic,

The patch 20ec628e8007: "misc: xilinx_sdfec: Add ability to configure
LDPC" from Jul 27, 2019, leads to the following static checker
warning:

	drivers/misc/xilinx_sdfec.c:504 xsdfec_reg1_write()
	warn: potential integer overflow from user 'no_packing << (10)'

drivers/misc/xilinx_sdfec.c
   492  static int xsdfec_reg1_write(struct xsdfec_dev *xsdfec, u32 psize,
   493                               u32 no_packing, u32 nm, u32 offset)
   494  {
   495          u32 wdata;
   496  
   497          if (psize < XSDFEC_REG1_PSIZE_MIN || psize > XSDFEC_REG1_PSIZE_MAX) {
   498                  dev_dbg(xsdfec->dev, "Psize is not in range");
   499                  return -EINVAL;
   500          }
   501  
   502          if (no_packing != 0 && no_packing != 1)
   503                  dev_dbg(xsdfec->dev, "No-packing bit register invalid");

Instead of writing invalid data, why not just return -EINVAL?

   504          no_packing = ((no_packing << XSDFEC_REG1_NO_PACKING_LSB) &
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Otherwise we have an integer overflow.

   505                        XSDFEC_REG1_NO_PACKING_MASK);
   506  
   507          if (nm & ~(XSDFEC_REG1_NM_MASK >> XSDFEC_REG1_NM_LSB))
   508                  dev_dbg(xsdfec->dev, "NM is beyond 10 bits");
   509          nm = (nm << XSDFEC_REG1_NM_LSB) & XSDFEC_REG1_NM_MASK;
   510  
   511          wdata = nm | no_packing | psize;
                             ^^^^^^^^^^
When I'm reviewing integer overflow warnings, I look to see if the
variable is re-used after the overflow, and this one is re-used here.
It's probably harmless but it's sort of a pain to review.

   512          if (XSDFEC_LDPC_CODE_REG1_ADDR_BASE + (offset * XSDFEC_LDPC_REG_JUMP) >
   513              XSDFEC_LDPC_CODE_REG1_ADDR_HIGH) {
   514                  dev_dbg(xsdfec->dev, "Writing outside of LDPC reg1 space 0x%x",
   515                          XSDFEC_LDPC_CODE_REG1_ADDR_BASE +
   516                                  (offset * XSDFEC_LDPC_REG_JUMP));
   517                  return -EINVAL;
   518          }
   519          xsdfec_regwrite(xsdfec,
   520                          XSDFEC_LDPC_CODE_REG1_ADDR_BASE +
   521                                  (offset * XSDFEC_LDPC_REG_JUMP),
   522                          wdata);
   523          return 0;
   524  }

See also:
drivers/misc/xilinx_sdfec.c:504 xsdfec_reg1_write() warn: potential integer overflow from user 'no_packing << (10)'
drivers/misc/xilinx_sdfec.c:509 xsdfec_reg1_write() warn: potential integer overflow from user 'nm << (11)'
drivers/misc/xilinx_sdfec.c:540 xsdfec_reg2_write() warn: potential integer overflow from user 'nmqc << (9)'
drivers/misc/xilinx_sdfec.c:544 xsdfec_reg2_write() warn: potential integer overflow from user 'norm_type << (20)'
drivers/misc/xilinx_sdfec.c:548 xsdfec_reg2_write() warn: potential integer overflow from user 'special_qc << (21)'
drivers/misc/xilinx_sdfec.c:554 xsdfec_reg2_write() warn: potential integer overflow from user 'no_final_parity << (22)'
drivers/misc/xilinx_sdfec.c:559 xsdfec_reg2_write() warn: potential integer overflow from user 'max_schedule << (23)'

regards,
dan carpenter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel