[PATCH v2 4/4] mtd: rawnand: brcmnand: Fix potential out-of-bounds access in oob write

William Zhang posted 4 patches 2 years, 7 months ago
There is a newer version of this series
[PATCH v2 4/4] mtd: rawnand: brcmnand: Fix potential out-of-bounds access in oob write
Posted by William Zhang 2 years, 7 months ago
When the oob buffer length is not in multiple of words, the oob write
function does out-of-bounds read on the oob source buffer at the last
iteration. Fix that by always checking length limit on the oob buffer
read and fill with 0xff when reaching the end of the buffer to the oob
registers.

Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")
Signed-off-by: William Zhang <william.zhang@broadcom.com>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>

---

Changes in v2:
- Handle the remaining unaligned oob data after the oob data write loop

 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index ea03104692bf..82e8434856c9 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1477,19 +1477,28 @@ static int write_oob_to_regs(struct brcmnand_controller *ctrl, int i,
 			     const u8 *oob, int sas, int sector_1k)
 {
 	int tbytes = sas << sector_1k;
-	int j;
+	int j, k = 0;
+	u32 last = 0xffffffff;
+	u8 *plast = (u8 *)&last;
 
 	/* Adjust OOB values for 1K sector size */
 	if (sector_1k && (i & 0x01))
 		tbytes = max(0, tbytes - (int)ctrl->max_oob);
 	tbytes = min_t(int, tbytes, ctrl->max_oob);
 
-	for (j = 0; j < tbytes; j += 4)
+	for (j = 0; (j + 3) < tbytes; j += 4)
 		oob_reg_write(ctrl, j,
 				(oob[j + 0] << 24) |
 				(oob[j + 1] << 16) |
 				(oob[j + 2] <<  8) |
 				(oob[j + 3] <<  0));
+
+	while (j < tbytes)
+		plast[k++] = oob[j++];
+
+	if (tbytes & 0x3)
+		oob_reg_write(ctrl, (tbytes & ~0x3), cpu_to_be32(last));
+
 	return tbytes;
 }
 
-- 
2.37.3

Re: [PATCH v2 4/4] mtd: rawnand: brcmnand: Fix potential out-of-bounds access in oob write
Posted by kernel test robot 2 years, 7 months ago
Hi William,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mtd/nand/next]
[also build test WARNING on linus/master v6.4-rc7 next-20230619]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/William-Zhang/mtd-rawnand-brcmnand-Fix-ECC-level-field-setting-for-v7-2-controller/20230617-103050
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next
patch link:    https://lore.kernel.org/r/20230617022920.67173-5-william.zhang%40broadcom.com
patch subject: [PATCH v2 4/4] mtd: rawnand: brcmnand: Fix potential out-of-bounds access in oob write
config: arm64-randconfig-s043-20230619 (https://download.01.org/0day-ci/archive/20230620/202306201340.C2Y4kmFL-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230620/202306201340.C2Y4kmFL-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306201340.C2Y4kmFL-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/mtd/nand/raw/brcmnand/brcmnand.c:1500:54: sparse: sparse: incorrect type in argument 3 (different base types) @@     expected unsigned int [usertype] data @@     got restricted __be32 [usertype] @@
   drivers/mtd/nand/raw/brcmnand/brcmnand.c:1500:54: sparse:     expected unsigned int [usertype] data
   drivers/mtd/nand/raw/brcmnand/brcmnand.c:1500:54: sparse:     got restricted __be32 [usertype]
   drivers/mtd/nand/raw/brcmnand/brcmnand.c:1836:42: sparse: sparse: cast to restricted __be32
   drivers/mtd/nand/raw/brcmnand/brcmnand.c:1836:42: sparse: sparse: cast to restricted __be32
   drivers/mtd/nand/raw/brcmnand/brcmnand.c:1836:42: sparse: sparse: cast to restricted __be32
   drivers/mtd/nand/raw/brcmnand/brcmnand.c:1836:42: sparse: sparse: cast to restricted __be32
   drivers/mtd/nand/raw/brcmnand/brcmnand.c:1836:42: sparse: sparse: cast to restricted __be32
   drivers/mtd/nand/raw/brcmnand/brcmnand.c:1836:42: sparse: sparse: cast to restricted __be32
   drivers/mtd/nand/raw/brcmnand/brcmnand.c:2065:41: sparse: sparse: dubious: x | !y

vim +1500 drivers/mtd/nand/raw/brcmnand/brcmnand.c

  1468	
  1469	/*
  1470	 * write_oob_to_regs - write data to OOB registers
  1471	 * @i: sub-page sector index
  1472	 * @oob: buffer to write from
  1473	 * @sas: spare area sector size (i.e., OOB size per FLASH_CACHE)
  1474	 * @sector_1k: 1 for 1KiB sectors, 0 for 512B, other values are illegal
  1475	 */
  1476	static int write_oob_to_regs(struct brcmnand_controller *ctrl, int i,
  1477				     const u8 *oob, int sas, int sector_1k)
  1478	{
  1479		int tbytes = sas << sector_1k;
  1480		int j, k = 0;
  1481		u32 last = 0xffffffff;
  1482		u8 *plast = (u8 *)&last;
  1483	
  1484		/* Adjust OOB values for 1K sector size */
  1485		if (sector_1k && (i & 0x01))
  1486			tbytes = max(0, tbytes - (int)ctrl->max_oob);
  1487		tbytes = min_t(int, tbytes, ctrl->max_oob);
  1488	
  1489		for (j = 0; (j + 3) < tbytes; j += 4)
  1490			oob_reg_write(ctrl, j,
  1491					(oob[j + 0] << 24) |
  1492					(oob[j + 1] << 16) |
  1493					(oob[j + 2] <<  8) |
  1494					(oob[j + 3] <<  0));
  1495	
  1496		while (j < tbytes)
  1497			plast[k++] = oob[j++];
  1498	
  1499		if (tbytes & 0x3)
> 1500			oob_reg_write(ctrl, (tbytes & ~0x3), cpu_to_be32(last));
  1501	
  1502		return tbytes;
  1503	}
  1504	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki