linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [bug report] misc: xilinx_sdfec: Add ability to configure LDPC
@ 2019-08-21  7:12 Dan Carpenter
  0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2019-08-21  7:12 UTC (permalink / raw)
  To: dragan.cvetic; +Cc: linux-arm-kernel

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:727 xsdfec_add_ldpc()
	warn: pointer comes from user 'ldpc->la_table'

drivers/misc/xilinx_sdfec.c
   647  static int xsdfec_add_ldpc(struct xsdfec_dev *xsdfec, void __user *arg)
   648  {
   649          struct xsdfec_ldpc_params *ldpc;
   650          int ret, n;
   651  
   652          ldpc = kzalloc(sizeof(*ldpc), GFP_KERNEL);
   653          if (!ldpc)
   654                  return -ENOMEM;
   655  
   656          if (copy_from_user(ldpc, arg, sizeof(*ldpc))) {
                                   ^^^^
ldpc comes from the user.

   657                  ret = -EFAULT;
   658                  goto err_out;
   659          }
   660  
   661          if (xsdfec->config.code == XSDFEC_TURBO_CODE) {
   662                  ret = -EIO;
   663                  goto err_out;
   664          }
   665  
   666          /* Verify Device has not started */
   667          if (xsdfec->state == XSDFEC_STARTED) {
   668                  ret = -EIO;
   669                  goto err_out;
   670          }
   671  
   672          if (xsdfec->config.code_wr_protect) {
   673                  ret = -EIO;
   674                  goto err_out;
   675          }
   676  
   677          /* Write Reg 0 */
   678          ret = xsdfec_reg0_write(xsdfec, ldpc->n, ldpc->k, ldpc->psize,
   679                                  ldpc->code_id);
   680          if (ret)
   681                  goto err_out;
   682  
   683          /* Write Reg 1 */
   684          ret = xsdfec_reg1_write(xsdfec, ldpc->psize, ldpc->no_packing, ldpc->nm,
   685                                  ldpc->code_id);
   686          if (ret)
   687                  goto err_out;
   688  
   689          /* Write Reg 2 */
   690          ret = xsdfec_reg2_write(xsdfec, ldpc->nlayers, ldpc->nmqc,
   691                                  ldpc->norm_type, ldpc->special_qc,
   692                                  ldpc->no_final_parity, ldpc->max_schedule,
   693                                  ldpc->code_id);
   694          if (ret)
   695                  goto err_out;
   696  
   697          /* Write Reg 3 */
   698          ret = xsdfec_reg3_write(xsdfec, ldpc->sc_off, ldpc->la_off,
   699                                  ldpc->qc_off, ldpc->code_id);
   700          if (ret)
   701                  goto err_out;
   702  
   703          /* Write Shared Codes */
   704          n = ldpc->nlayers / 4;
   705          if (ldpc->nlayers % 4)
   706                  n++;
   707  
   708          ret = xsdfec_table_write(xsdfec, ldpc->sc_off, ldpc->sc_table, n,
                                                               ^^^^^^^^^^^^^^
This is not a bug, but it's more like an aesthetic thing.  I feel like
->sc_table should be tagged as a __user pointer, but I'm not sure of
the rules exactly.  Also the comments say it has to be page aligned but
it will I don't think anyone checks and it should work fine either way
because it just gets rounded down.

   709                                   XSDFEC_LDPC_SC_TABLE_ADDR_BASE,
   710                                   XSDFEC_SC_TABLE_DEPTH);
   711          if (ret < 0)
   712                  goto err_out;
   713  
   714          ret = xsdfec_table_write(xsdfec, 4 * ldpc->la_off, ldpc->la_table,
   715                                   ldpc->nlayers, XSDFEC_LDPC_LA_TABLE_ADDR_BASE,
   716                                   XSDFEC_LA_TABLE_DEPTH);
   717          if (ret < 0)
   718                  goto err_out;
   719  
   720          ret = xsdfec_table_write(xsdfec, 4 * ldpc->qc_off, ldpc->qc_table,
   721                                   ldpc->nqc, XSDFEC_LDPC_QC_TABLE_ADDR_BASE,
   722                                   XSDFEC_QC_TABLE_DEPTH);
   723          if (ret > 0)
   724                  ret = 0;
   725  err_out:
   726          kfree(ldpc);
   727          return ret;
   728  }

regards,
dan carpenter

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [bug report] misc: xilinx_sdfec: Add ability to configure LDPC
@ 2019-08-21  7:13 Dan Carpenter
  0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2019-08-21  7:13 UTC (permalink / raw)
  To: dragan.cvetic; +Cc: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-08-21  7:16 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-21  7:12 [bug report] misc: xilinx_sdfec: Add ability to configure LDPC Dan Carpenter
2019-08-21  7:13 Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).